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

feat: Add ESM support for common and common-client (rollup) #604

Merged
merged 22 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ yarn-error.log
.vscode
dump.rdb
.wrangler
stats.html
Copy link
Member Author

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.

Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,7 @@ export class ClientEntity {
if (!identifyParams) {
throw malformedCommand;
}
await this.client.identify(identifyParams.user || identifyParams.context, {
waitForNetworkResults: true,
});
await this.client.identify(identifyParams.user || identifyParams.context);
return undefined;
}

Expand Down Expand Up @@ -204,7 +202,7 @@ export async function newSdkClientEntity(options: CreateInstanceParams) {
let failed = false;
try {
await Promise.race([
client.identify(initialContext, { waitForNetworkResults: true }),
client.identify(initialContext),
new Promise((_resolve, reject) => {
setTimeout(reject, timeout);
}),
Expand Down
3 changes: 0 additions & 3 deletions packages/sdk/browser/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@ export default {
verbose: true,
preset: 'ts-jest/presets/default-esm',
testEnvironment: 'jest-environment-jsdom',
transform: {
Copy link
Member Author

Choose a reason for hiding this comment

The 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'],
};
3 changes: 2 additions & 1 deletion packages/sdk/browser/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@
},
"dependencies": {
"@launchdarkly/js-client-sdk-common": "1.8.0",
"escape-string-regexp": "^5.0.0"
"escape-string-regexp": "^5.0.0",
"rollup-plugin-visualizer": "^5.12.0"
},
"devDependencies": {
"@jest/globals": "^29.7.0",
Expand Down
3 changes: 3 additions & 0 deletions packages/sdk/browser/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ 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';
import { visualizer } from 'rollup-plugin-visualizer';

const getSharedConfig = (format, file) => ({
input: 'src/index.ts',
Expand Down Expand Up @@ -34,6 +35,8 @@ export default [
resolve(),
terser(),
json(),
// The 'sourcemap' option allows using the minified size, not the size before minification.
visualizer({ sourcemap: true }),
],
},
{
Expand Down
6 changes: 3 additions & 3 deletions packages/sdk/browser/src/BrowserClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import {
LDClientImpl,
LDContext,
LDEmitter,
LDEmitterEventName,
Copy link
Member Author

Choose a reason for hiding this comment

The 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 EventName name was already used.

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';
Expand Down Expand Up @@ -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();
}
Expand Down
2 changes: 1 addition & 1 deletion packages/sdk/browser/src/goals/GoalTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typescript spontaneously became more strict about this.


pageviewGoals.forEach((event) => onEvent(event));

Expand Down
3 changes: 1 addition & 2 deletions packages/sdk/browser/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@
"allowSyntheticDefaultImports": true,
"declaration": true,
"declarationMap": true,
"jsx": "react-jsx",
"lib": ["ES2017", "dom"],
"module": "ES6",
"module": "ESNext",
"moduleResolution": "node",
"noImplicitOverride": true,
"resolveJsonModule": true,
Expand Down
22 changes: 17 additions & 5 deletions packages/shared/common/package.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"name": "@launchdarkly/js-sdk-common",
"version": "2.9.0",
"type": "commonjs",
"main": "./dist/index.js",
"type": "module",
"main": "./dist/index.mjs",
"types": "./dist/index.d.ts",
"homepage": "https://github.com/launchdarkly/js-core/tree/main/packages/shared/common",
"repository": {
Expand All @@ -18,18 +18,27 @@
"analytics",
"client"
],
"exports": {
"types": "./dist/index.d.ts",
"require": "./dist/index.cjs",
"import": "./dist/index.mjs"
},
"scripts": {
"test": "npx jest --ci",
"build-types": "npx tsc --declaration true --emitDeclarationOnly true --declarationDir dist",
"build": "npx tsc",
"clean": "npx tsc --build --clean",
"build": "npx tsc --noEmit && rollup -c rollup.config.js",
"clean": "rimraf dist",
"lint": "npx eslint . --ext .ts",
"lint:fix": "yarn run lint --fix",
"prettier": "prettier --write 'src/*.@(js|ts|tsx|json)'",
"check": "yarn && yarn prettier && yarn lint && tsc && yarn test"
},
"license": "Apache-2.0",
"devDependencies": {
"@rollup/plugin-commonjs": "^25.0.0",
"@rollup/plugin-json": "^6.1.0",
"@rollup/plugin-node-resolve": "^15.0.2",
"@rollup/plugin-terser": "^0.4.3",
"@rollup/plugin-typescript": "^11.1.1",
"@trivago/prettier-plugin-sort-imports": "^4.1.1",
"@types/jest": "^29.4.0",
"@typescript-eslint/eslint-plugin": "^6.20.0",
Expand All @@ -44,7 +53,10 @@
"jest": "^29.5.0",
"launchdarkly-js-test-helpers": "^2.2.0",
"prettier": "^3.0.0",
"rimraf": "6.0.1",
"rollup": "^3.23.0",
"ts-jest": "^29.0.5",
"tslib": "^2.7.0",
"typedoc": "0.25.0",
"typescript": "5.1.6"
}
Expand Down
22 changes: 10 additions & 12 deletions packages/shared/common/rollup.config.js
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: [
Expand All @@ -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'),
Copy link
Member Author

Choose a reason for hiding this comment

The 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()],
},
];
6 changes: 4 additions & 2 deletions packages/shared/common/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
"compilerOptions": {
"rootDir": "src",
"outDir": "dist",
"target": "ES2017",
"target": "ES2020",
Copy link
Member Author

Choose a reason for hiding this comment

The 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
Expand All @@ -15,5 +16,6 @@
"stripInternal": true,
"composite": true
},
"include": ["src"],
"exclude": ["**/*.test.ts", "dist", "node_modules", "__tests__"]
}
20 changes: 16 additions & 4 deletions packages/shared/sdk-client/package.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"name": "@launchdarkly/js-client-sdk-common",
"version": "1.8.0",
"type": "commonjs",
"main": "./dist/index.js",
"type": "module",
"main": "./dist/index.mjs",
"types": "./dist/index.d.ts",
"homepage": "https://github.com/launchdarkly/js-core/tree/main/packages/shared/sdk-client",
"repository": {
Expand All @@ -18,11 +18,16 @@
"analytics",
"client"
],
"exports": {
"types": "./dist/index.d.ts",
"require": "./dist/index.cjs",
"import": "./dist/index.mjs"
},
"scripts": {
"doc": "../../../scripts/build-doc.sh .",
"test": "npx jest --ci",
"build": "npx tsc",
"clean": "npx tsc --build --clean",
"build": "tsc --noEmit && rollup -c rollup.config.js",
"clean": "rimraf dist",
"lint": "npx eslint . --ext .ts",
"lint:fix": "yarn run lint -- --fix",
"prettier": "prettier --write 'src/*.@(js|ts|tsx|json)'",
Expand All @@ -33,6 +38,11 @@
"@launchdarkly/js-sdk-common": "2.9.0"
},
"devDependencies": {
"@rollup/plugin-commonjs": "^25.0.0",
"@rollup/plugin-json": "^6.1.0",
"@rollup/plugin-node-resolve": "^15.0.2",
"@rollup/plugin-terser": "^0.4.3",
"@rollup/plugin-typescript": "^11.1.1",
"@testing-library/dom": "^9.3.1",
"@testing-library/jest-dom": "^5.16.5",
"@types/jest": "^29.5.3",
Expand All @@ -51,6 +61,8 @@
"jest-environment-jsdom": "^29.6.1",
"launchdarkly-js-test-helpers": "^2.2.0",
"prettier": "^3.0.0",
"rimraf": "6.0.1",
"rollup": "^3.23.0",
"ts-jest": "^29.1.1",
"typedoc": "0.25.0",
"typescript": "5.1.6"
Expand Down
20 changes: 10 additions & 10 deletions packages/shared/sdk-client/rollup.config.js
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()],
},
];
3 changes: 2 additions & 1 deletion packages/shared/sdk-client/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { LDClientInternalOptions } from './configuration/Configuration';
import DataSourceStatus, { DataSourceState } from './datasource/DataSourceStatus';
import DataSourceStatusErrorInfo from './datasource/DataSourceStatusErrorInfo';
import LDClientImpl from './LDClientImpl';
import LDEmitter from './LDEmitter';
import LDEmitter, { EventName } from './LDEmitter';
import Requestor from './polling/Requestor';

export * from '@launchdarkly/js-sdk-common';
Expand Down Expand Up @@ -38,4 +38,5 @@ export {
LDClientImpl,
LDClientInternalOptions,
DataSourceState,
EventName as LDEmitterEventName,
};
5 changes: 3 additions & 2 deletions packages/shared/sdk-client/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
"compilerOptions": {
"rootDir": "src",
"outDir": "dist",
"target": "ES2017",
"target": "ES2020",
"lib": ["es6", "DOM"],
"module": "commonjs",
"module": "ESNext",
"moduleResolution": "node",
"strict": true,
"noImplicitOverride": true,
// Needed for CommonJS modules: markdown-it, fs-extra
Expand Down
Loading