Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TypeScript support #104

Merged
merged 3 commits into from
May 7, 2024
Merged

Conversation

CharlesMangwa
Copy link
Contributor

Motivation 💪

We're planning on using Emarsys at colorfy for some of our projects. However, the lack of static typing is a major hindrance for us, as all of them are fully typed. That what birthed this PR with the goal to implement support for TypeScript.

Approach 🤔

While making this PR, we tried to keep in mind the desire not to disturb how the library has been built so far. That's why we decided to keep the library written in JavaScript and simply make use of the existing comments to generate the equivalent TypeScript .d.ts declaration files.

Note

Doing so is possible thanks to a feature introduced in TypeScript 3.7. Our process described below was based on a TypeScript official guide.

Therefore, our approach was to:

  1. Slightly modify the current comments in order to have them comply to the JSDoc standards.
  2. Set up a TypeScript config file that the compiler will use to generate the .d.ts declaration files.
  3. Add an npm script that will automatically compile the TypeScript files before each release package is packed.

The main advantage of proceeding as such is that you will not have to modify anything major in how you were building this library up until now.

Warning

The only noticeable change is a rather small one: you'll just have to make sure to comply to the JSDoc standards when writting comments from now on.

Hopefully, this won't be much of a chore as the current comments were already really close to it.

Eg:

// FROM:

/**
 * @desc The Emarsys SDK automatically handles setPushToken for the device and it is recommended to leave this to the SDK.
 * However if you have your custom implementation of MessagingService, please use the setPushToken() method.
 * @param required string pushToken - Push Token of your MessagingService
 * @return bool - success or failure
 */
setPushToken(pushToken) {
  return RNEmarsysPushWrapper.setPushToken(pushToken);
}

// TO:

/**
 * The Emarsys SDK automatically handles `setPushToken()` for the device and it is recommended to leave this to the SDK.
 * However if you have your custom implementation of `MessagingService`, please use the `setPushToken()` method.
 * @param {string} pushToken - Push Token of your `MessagingService`.
 * @returns {Promise<boolean>} Promise with success or failure boolean.
 */
setPushToken(pushToken) {
  return RNEmarsysPushWrapper.setPushToken(pushToken);
}

Disclaimers 📣

  1. While performing the change, we also took the liberty to fix some small typos here and there or comments that were not accurate. Eg:
  /**
+   * @param {number} contactFieldId - Field used for identification.
+   * @param {string} contactFieldValue - User identification.
-   * @param required string contactFieldValue - user identification
-   * @param required integer contactFieldId - field used for identification
  setContact(contactFieldId, contactFieldValue) {
    return RNEmarsysWrapper.setContact(contactFieldId, contactFieldValue);
  },
  1. We also marked in the JSDoc comments all the Predict.recommend...() methods that are @deprecated in a way that the compiled TypeScript interfaces will complain at any further use, even accidental. A link to the new Predict.recommendProducts() method was also added.

  2. Incidentally, we also marked Push.pushToken() as @deprecated and duplicated it into a new Push.getPushToken(). This naming seemed more logical in the context of the other existing methods: Push.setPushToken() & Push.clearPushToken(). Of course, no functional change was made to the existing Push.pushToken() while doing so.

Build 🏗️

The build process has received very minimal modifications.

Important

The sole new requirement is for your machines to have TypeScript's tsc compiler installed. This can be done in various ways; the one this PR implements is adding TypeScript as a devDependencies.

From there, any time the package is being packed (or the command tsc is being run in the root folder), the compiler will be looking at 2 paths: ./index.js & ./src/**/* (as instructed in ./tsconfig.json#L3).

If it finds any JavaScript files there, all the .d.ts declaration files and their respective source maps will be generated in a newly created ./typescript folder.

When the library is consumed by a client, the new "types": "typescript/src/index.d.ts" line in the package.json will let TypeScript-enabled projects know where to find the declaration files.

Tip

Once again, just make sure to adhere to the JSDoc standards to keep the process running smoothly even when adding new APIs.

This is a mandatory steps to add support for TypeScript without migrating the entire library.

More details: https://jsdoc.app.
@biancalui-emarsys
Copy link
Contributor

Hi @CharlesMangwa ,

Thanks a lot for implementing support for TypeScript and creating this PR.

However, when we try to npm install for our sample project https://github.com/emartech/react-native-emarsys-sdk/tree/master/sample with dependency "react-native-emarsys-wrapper": "github:colorfy-software/react-native-emarsys-sdk" , we encounter the following error.

sample % npm i
npm WARN deprecated @babel/[email protected]: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-class-properties instead.
npm WARN deprecated @babel/[email protected]: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-nullish-coalescing-operator instead.
npm WARN deprecated @babel/[email protected]: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-optional-catch-binding instead.
npm WARN deprecated @babel/[email protected]: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-optional-chaining instead.
npm WARN deprecated @babel/[email protected]: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-async-generator-functions instead.
npm WARN deprecated @babel/[email protected]: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-object-rest-spread instead.
npm WARN deprecated [email protected]: babel-eslint is now @babel/eslint-parser. This package will no longer receive updates.
npm WARN deprecated [email protected]: support for ECMAScript is superseded by `uglify-js` as of v3.13.0
npm ERR! code 2
npm ERR! git dep preparation failed
npm ERR! command /Users/i/.nvm/versions/node/v18.19.0/bin/node /Users/i/.nvm/versions/node/v18.19.0/lib/node_modules/npm/bin/npm-cli.js install --force --cache=/Users/i/.npm --prefer-offline=false --prefer-online=false --offline=false --no-progress --no-save --no-audit --include=dev --include=peer --include=optional --no-package-lock-only --no-dry-run
npm ERR! > [email protected] prepare
npm ERR! > tsc
npm ERR! 
npm ERR! node_modules/react-native/Libraries/Components/DrawerAndroid/DrawerLayoutAndroid.d.ts(108,31): error TS2503: Cannot find namespace 'JSX'.
npm ERR! node_modules/react-native/Libraries/Lists/FlatList.d.ts(225,29): error TS2503: Cannot find namespace 'JSX'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(78,13): error TS2403: Subsequent variable declarations must have the same type.  Variable 'Blob' must be of type '{ new (blobParts?: BlobPart[], options?: BlobPropertyBag): Blob; prototype: Blob; }', but here has type '{ new (blobParts?: (string | Blob)[], options?: BlobOptions): Blob; prototype: Blob; }'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(84,3): error TS2687: All declarations of 'name' must have identical modifiers.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(85,3): error TS2687: All declarations of 'lastModified' must have identical modifiers.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(88,13): error TS2403: Subsequent variable declarations must have the same type.  Variable 'File' must be of type '{ new (fileBits: BlobPart[], fileName: string, options?: FilePropertyBag): File; prototype: File; }', but here has type '{ new (fileParts?: (string | Blob)[], name?: string, options?: BlobOptions): File; prototype: File; }'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(113,15): error TS2300: Duplicate identifier 'FormData'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(137,13): error TS2403: Subsequent variable declarations must have the same type.  Variable 'Headers' must be of type '{ new (init?: HeadersInit): Headers; prototype: Headers; }', but here has type '{ new (init?: HeadersInit_): Headers; prototype: Headers; }'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(171,3): error TS2717: Subsequent property declarations must have the same type.  Property 'body' must be of type 'BodyInit', but here has type 'BodyInit_'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(173,3): error TS2717: Subsequent property declarations must have the same type.  Property 'headers' must be of type 'HeadersInit', but here has type 'HeadersInit_'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(179,3): error TS2717: Subsequent property declarations must have the same type.  Property 'window' must be of type 'null', but here has type 'any'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(194,13): error TS2403: Subsequent variable declarations must have the same type.  Variable 'Request' must be of type '{ new (input: RequestInfo | URL, init?: RequestInit): Request; prototype: Request; }', but here has type '{ new (input: string | Request, init?: RequestInit): Request; prototype: Request; }'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(199,14): error TS2300: Duplicate identifier 'RequestInfo'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(202,3): error TS2717: Subsequent property declarations must have the same type.  Property 'headers' must be of type 'HeadersInit', but here has type 'HeadersInit_'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(218,13): error TS2403: Subsequent variable declarations must have the same type.  Variable 'Response' must be of type '{ new (body?: BodyInit, init?: ResponseInit): Response; prototype: Response; error(): Response; json(data: any, init?: ResponseInit): Response; redirect(url: string | URL, status?: number): Response; }', but here has type '{ new (body?: BodyInit_, init?: ResponseInit): Response; prototype: Response; error: () => Response; redirect: (url: string, status?: number) => Response; }'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(308,3): error TS2717: Subsequent property declarations must have the same type.  Property 'abort' must be of type 'ProgressEvent<XMLHttpRequestEventTarget>', but here has type 'ProgressEvent<EventTarget>'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(309,3): error TS2717: Subsequent property declarations must have the same type.  Property 'error' must be of type 'ProgressEvent<XMLHttpRequestEventTarget>', but here has type 'ProgressEvent<EventTarget>'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(310,3): error TS2717: Subsequent property declarations must have the same type.  Property 'load' must be of type 'ProgressEvent<XMLHttpRequestEventTarget>', but here has type 'ProgressEvent<EventTarget>'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(311,3): error TS2717: Subsequent property declarations must have the same type.  Property 'loadend' must be of type 'ProgressEvent<XMLHttpRequestEventTarget>', but here has type 'ProgressEvent<EventTarget>'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(312,3): error TS2717: Subsequent property declarations must have the same type.  Property 'loadstart' must be of type 'ProgressEvent<XMLHttpRequestEventTarget>', but here has type 'ProgressEvent<EventTarget>'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(313,3): error TS2717: Subsequent property declarations must have the same type.  Property 'progress' must be of type 'ProgressEvent<XMLHttpRequestEventTarget>', but here has type 'ProgressEvent<EventTarget>'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(314,3): error TS2717: Subsequent property declarations must have the same type.  Property 'timeout' must be of type 'ProgressEvent<XMLHttpRequestEventTarget>', but here has type 'ProgressEvent<EventTarget>'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(367,14): error TS2300: Duplicate identifier 'XMLHttpRequestResponseType'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(380,15): error TS2300: Duplicate identifier 'URL'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(405,15): error TS2300: Duplicate identifier 'URLSearchParams'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(464,3): error TS2717: Subsequent property declarations must have the same type.  Property 'onopen' must be of type '(this: WebSocket, ev: Event) => any', but here has type '() => void'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(465,3): error TS2717: Subsequent property declarations must have the same type.  Property 'onmessage' must be of type '(this: WebSocket, ev: MessageEvent<any>) => any', but here has type '(event: WebSocketMessageEvent) => void'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(466,3): error TS2717: Subsequent property declarations must have the same type.  Property 'onerror' must be of type '(this: WebSocket, ev: Event) => any', but here has type '(event: WebSocketErrorEvent) => void'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(467,3): error TS2717: Subsequent property declarations must have the same type.  Property 'onclose' must be of type '(this: WebSocket, ev: CloseEvent) => any', but here has type '(event: WebSocketCloseEvent) => void'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(468,3): error TS2717: Subsequent property declarations must have the same type.  Property 'addEventListener' must be of type '{ <K extends keyof WebSocketEventMap>(type: K, listener: (this: WebSocket, ev: WebSocketEventMap[K]) => any, options?: boolean | AddEventListenerOptions): void; (type: string, listener: EventListenerOrEventListenerObject, options?: boolean | AddEventListenerOptions): void; }', but here has type 'WebsocketEventListener'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(469,3): error TS2717: Subsequent property declarations must have the same type.  Property 'removeEventListener' must be of type '{ <K extends keyof WebSocketEventMap>(type: K, listener: (this: WebSocket, ev: WebSocketEventMap[K]) => any, options?: boolean | EventListenerOptions): void; (type: string, listener: EventListenerOrEventListenerObject, options?: boolean | EventListenerOptions): void; }', but here has type 'WebsocketEventListener'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(472,13): error TS2403: Subsequent variable declarations must have the same type.  Variable 'WebSocket' must be of type '{ new (url: string | URL, protocols?: string | string[]): WebSocket; prototype: WebSocket; readonly CONNECTING: 0; readonly OPEN: 1; readonly CLOSING: 2; readonly CLOSED: 3; }', but here has type '{ new (uri: string, protocols?: string | string[], options?: { [optionName: string]: any; headers: { [headerName: string]: string; }; }): WebSocket; prototype: WebSocket; readonly CLOSED: number; readonly CLOSING: number; readonly CONNECTING: number; readonly OPEN: number; }'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(496,15): error TS2300: Duplicate identifier 'AbortSignal'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(496,15): error TS2420: Class 'AbortSignal' incorrectly implements interface 'EventTarget'.
npm ERR!   Property 'dispatchEvent' is missing in type 'AbortSignal' but required in type 'EventTarget'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(531,15): error TS2300: Duplicate identifier 'AbortController'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(556,12): error TS2717: Subsequent property declarations must have the same type.  Property 'error' must be of type 'DOMException', but here has type 'Error'.
npm ERR! node_modules/react-native/types/modules/globals.d.ts(565,12): error TS2717: Subsequent property declarations must have the same type.  Property 'readyState' must be of type '0 | 2 | 1', but here has type 'number'.
npm ERR! node_modules/typescript/lib/lib.dom.d.ts(2289,11): error TS2300: Duplicate identifier 'AbortController'.
npm ERR! node_modules/typescript/lib/lib.dom.d.ts(2304,13): error TS2300: Duplicate identifier 'AbortController'.
npm ERR! node_modules/typescript/lib/lib.dom.d.ts(2318,11): error TS2300: Duplicate identifier 'AbortSignal'.
npm ERR! node_modules/typescript/lib/lib.dom.d.ts(2337,13): error TS2300: Duplicate identifier 'AbortSignal'.
npm ERR! node_modules/typescript/lib/lib.dom.d.ts(8309,14): error TS2687: All declarations of 'lastModified' must have identical modifiers.
npm ERR! node_modules/typescript/lib/lib.dom.d.ts(8311,14): error TS2687: All declarations of 'name' must have identical modifiers.
npm ERR! node_modules/typescript/lib/lib.dom.d.ts(8657,11): error TS2300: Duplicate identifier 'FormData'.
npm ERR! node_modules/typescript/lib/lib.dom.d.ts(8677,13): error TS2300: Duplicate identifier 'FormData'.
npm ERR! node_modules/typescript/lib/lib.dom.d.ts(22703,11): error TS2300: Duplicate identifier 'URL'.
npm ERR! node_modules/typescript/lib/lib.dom.d.ts(22733,13): error TS2300: Duplicate identifier 'URL'.
npm ERR! node_modules/typescript/lib/lib.dom.d.ts(22748,11): error TS2300: Duplicate identifier 'URLSearchParams'.
npm ERR! node_modules/typescript/lib/lib.dom.d.ts(22794,13): error TS2300: Duplicate identifier 'URLSearchParams'.
npm ERR! node_modules/typescript/lib/lib.dom.d.ts(28229,6): error TS2300: Duplicate identifier 'RequestInfo'.
npm ERR! node_modules/typescript/lib/lib.dom.d.ts(28418,6): error TS2300: Duplicate identifier 'XMLHttpRequestResponseType'.
npm ERR! npm WARN using --force Recommended protections disabled.
npm ERR! npm WARN deprecated @babel/[email protected]: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-numeric-separator instead.
npm ERR! npm WARN deprecated @babel/[email protected]: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-optional-catch-binding instead.
npm ERR! npm WARN deprecated @babel/[email protected]: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-nullish-coalescing-operator instead.
npm ERR! npm WARN deprecated @babel/[email protected]: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-class-properties instead.
npm ERR! npm WARN deprecated @babel/[email protected]: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-logical-assignment-operators instead.
npm ERR! npm WARN deprecated @babel/[email protected]: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-async-generator-functions instead.
npm ERR! npm WARN deprecated @babel/[email protected]: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-optional-chaining instead.
npm ERR! npm WARN deprecated @babel/[email protected]: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-object-rest-spread instead.
npm ERR! npm WARN deprecated [email protected]: The querystring API is considered Legacy. new code should use the URLSearchParams API instead.
npm ERR! npm ERR! code 2
npm ERR! npm ERR! path /Users/i/.npm/_cacache/tmp/git-clone6uPMD6
npm ERR! npm ERR! command failed
npm ERR! npm ERR! command sh -c tsc
npm ERR! 
npm ERR! npm ERR! A complete log of this run can be found in: /Users/i/.npm/_logs/2024-04-30T04_56_51_917Z-debug-0.log

npm ERR! A complete log of this run can be found in: /Users/i/.npm/_logs/2024-04-30T04_56_43_619Z-debug-0.log
sample % 

May you fix this?

@CharlesMangwa
Copy link
Contributor Author

Hi @biancalui-emarsys!

Thanks for the feedback!

It was an oversight on my end which should be resolved now:

# cd sample && npm i 
added 1243 packages, and audited 1245 packages in 9s

143 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

@biancalui-emarsys
Copy link
Contributor

Hi @CharlesMangwa ,

It is working now when using dependency
"react-native-emarsys-wrapper": "github:colorfy-software/react-native-emarsys-sdk"

Unfortunately using the package as local module doesn't work on machines that do not have TypeScript installed globally.
"react-native-emarsys-wrapper": "file:.."

sample % npm i
npm ERR! code 127
npm ERR! path /Users/i/B/react-native-emarsys-sdk-colorfy
npm ERR! command failed
npm ERR! command sh -c tsc
npm ERR! sh: tsc: command not found

It might be better to revert the last commit and keep sample project as is.

@CharlesMangwa
Copy link
Contributor Author

Hey @biancalui-emarsys!

Thanks for the heads up, change removed! ✅

@biancalui-emarsys biancalui-emarsys merged commit 518f83a into emartech:master May 7, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants