-
Notifications
You must be signed in to change notification settings - Fork 18
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
feat: Add ESM support for common and common-client (rollup) #604
Changes from all commits
5e2bd0b
f449f1d
c9b0a1d
1b21500
cec3bde
9736e18
699fe35
065f83e
1e60696
ec98330
bbde020
9e1784c
676477a
17cba84
c85014b
b7d585d
6feb546
000dfbd
fd693b6
cd882d7
72c27cc
e4d02ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,3 +21,4 @@ yarn-error.log | |
.vscode | ||
dump.rdb | ||
.wrangler | ||
stats.html | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,9 +3,6 @@ export default { | |
verbose: true, | ||
preset: 'ts-jest/presets/default-esm', | ||
testEnvironment: 'jest-environment-jsdom', | ||
transform: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The preset takes care of this and works now. |
||
'^.+\\.tsx?$': ['ts-jest', { useESM: true, tsconfig: 'tsconfig.json' }], | ||
}, | ||
testPathIgnorePatterns: ['./dist', './src'], | ||
testMatch: ['**.test.ts'], | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,10 +10,10 @@ import { | |
LDClientImpl, | ||
LDContext, | ||
LDEmitter, | ||
LDEmitterEventName, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. EventName was being imported inside a 'dist' which is a curse of VSCode auto imports. Now the es modules don't allow that (hurray). So it needed fixed and the |
||
LDHeaders, | ||
Platform, | ||
} from '@launchdarkly/js-client-sdk-common'; | ||
import { EventName } from '@launchdarkly/js-client-sdk-common/dist/LDEmitter'; | ||
|
||
import BrowserDataManager from './BrowserDataManager'; | ||
import { BrowserIdentifyOptions as LDIdentifyOptions } from './BrowserIdentifyOptions'; | ||
|
@@ -233,12 +233,12 @@ export class BrowserClient extends LDClientImpl implements LDClient { | |
browserDataManager.setAutomaticStreamingState(!!this.emitter.listenerCount('change')); | ||
} | ||
|
||
override on(eventName: EventName, listener: Function): void { | ||
override on(eventName: LDEmitterEventName, listener: Function): void { | ||
super.on(eventName, listener); | ||
this.updateAutomaticStreamingState(); | ||
} | ||
|
||
override off(eventName: EventName, listener: Function): void { | ||
override off(eventName: LDEmitterEventName, listener: Function): void { | ||
super.off(eventName, listener); | ||
this.updateAutomaticStreamingState(); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,7 +73,7 @@ export default class GoalTracker { | |
); | ||
|
||
const pageviewGoals = goalsMatchingUrl.filter((goal) => goal.kind === 'pageview'); | ||
const clickGoals = goalsMatchingUrl.filter((goal) => goal.kind === 'click'); | ||
const clickGoals = goalsMatchingUrl.filter((goal) => goal.kind === 'click') as ClickGoal[]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typescript spontaneously became more strict about this. |
||
|
||
pageviewGoals.forEach((event) => onEvent(event)); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,12 @@ | ||
import common from '@rollup/plugin-commonjs'; | ||
import json from '@rollup/plugin-json'; | ||
import resolve from '@rollup/plugin-node-resolve'; | ||
import terser from '@rollup/plugin-terser'; | ||
import typescript from '@rollup/plugin-typescript'; | ||
|
||
// The common library does not have a dependency resolution plugin as it should not have any | ||
// dependencies. | ||
|
||
// This library is not minified as the final SDK package is responsible for minification. | ||
|
||
const getSharedConfig = (format, file) => ({ | ||
input: 'src/index.ts', | ||
output: [ | ||
|
@@ -13,31 +16,26 @@ const getSharedConfig = (format, file) => ({ | |
file: file, | ||
}, | ||
], | ||
onwarn: (warning) => { | ||
if (warning.code !== 'CIRCULAR_DEPENDENCY') { | ||
console.error(`(!) ${warning.message}`); | ||
} | ||
}, | ||
}); | ||
|
||
export default [ | ||
{ | ||
...getSharedConfig('es', 'dist/index.es.js'), | ||
...getSharedConfig('es', 'dist/index.mjs'), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To appease jest. |
||
plugins: [ | ||
typescript({ | ||
module: 'esnext', | ||
tsconfig: './tsconfig.json', | ||
outputToFilesystem: true, | ||
}), | ||
common({ | ||
transformMixedEsModules: true, | ||
esmExternals: true, | ||
}), | ||
resolve(), | ||
terser(), | ||
json(), | ||
], | ||
}, | ||
{ | ||
...getSharedConfig('cjs', 'dist/index.cjs.js'), | ||
plugins: [typescript(), common(), resolve(), terser(), json()], | ||
...getSharedConfig('cjs', 'dist/index.cjs'), | ||
plugins: [typescript({ tsconfig: './tsconfig.json' }), common(), json()], | ||
}, | ||
]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,10 @@ | |
"compilerOptions": { | ||
"rootDir": "src", | ||
"outDir": "dist", | ||
"target": "ES2017", | ||
"target": "ES2020", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So that the spread operator for object literals will not be transformed. Helps with size of the web bundle. |
||
"lib": ["es6"], | ||
"module": "commonjs", | ||
"module": "ESNext", | ||
"moduleResolution": "node", | ||
"strict": true, | ||
"noImplicitOverride": true, | ||
// Needed for CommonJS modules: markdown-it, fs-extra | ||
|
@@ -15,5 +16,6 @@ | |
"stripInternal": true, | ||
"composite": true | ||
}, | ||
"include": ["src"], | ||
"exclude": ["**/*.test.ts", "dist", "node_modules", "__tests__"] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,43 +1,43 @@ | ||
import common from '@rollup/plugin-commonjs'; | ||
import json from '@rollup/plugin-json'; | ||
import resolve from '@rollup/plugin-node-resolve'; | ||
import terser from '@rollup/plugin-terser'; | ||
import typescript from '@rollup/plugin-typescript'; | ||
|
||
// This library is not minified as the final SDK package is responsible for minification. | ||
|
||
const getSharedConfig = (format, file) => ({ | ||
input: 'src/index.ts', | ||
// Intermediate modules don't bundle all dependencies. We leave that to leaf-node | ||
// SDK implementations. | ||
external: ['@launchdarkly/js-sdk-common'], | ||
output: [ | ||
{ | ||
format: format, | ||
sourcemap: true, | ||
file: file, | ||
}, | ||
], | ||
onwarn: (warning) => { | ||
if (warning.code !== 'CIRCULAR_DEPENDENCY') { | ||
console.error(`(!) ${warning.message}`); | ||
} | ||
}, | ||
}); | ||
|
||
export default [ | ||
{ | ||
...getSharedConfig('es', 'dist/index.es.js'), | ||
...getSharedConfig('es', 'dist/index.mjs'), | ||
plugins: [ | ||
typescript({ | ||
module: 'esnext', | ||
tsconfig: './tsconfig.json', | ||
outputToFilesystem: true, | ||
}), | ||
common({ | ||
transformMixedEsModules: true, | ||
esmExternals: true, | ||
}), | ||
resolve(), | ||
terser(), | ||
json(), | ||
], | ||
}, | ||
{ | ||
...getSharedConfig('cjs', 'dist/index.cjs.js'), | ||
plugins: [typescript(), common(), resolve(), terser(), json()], | ||
...getSharedConfig('cjs', 'dist/index.cjs'), | ||
plugins: [typescript({ tsconfig: './tsconfig.json' }), common(), resolve(), json()], | ||
}, | ||
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added for analyzing bundle size.