refactor: Convert entry points to es module and create minified bundles #445

fayyazarshad · 2020-04-07T05:30:07Z

Summary

This is the first step in an effort to convert javascript SDK from Common JS to ES Module. The changes include

Convert all the entry point files to ES Module.
Minified Common JS bundles are being created for each entry point using Rollup and Terser
main: dist/optimizely.node.min.js
browser: dist/optimizely.browser.min.js
react-native: dist/optimizely.react_native.min.js
All the bundles including umd bundle are generated using Rollup and terser. Webpack is only being used to keep the Karma tests running.

Why Webpack for Karma?

We tried using Rollup with Karma for the same purpose but couldnt run it despite trying many times. To not block progress on the ES module conversion, we recommend moving forward with using webpack to keep running karma tests when all the bundles are being created by Rollup and terser. We can revisit and modify Karma to use Rollup later.

Note about Testapp

This PR uses a specific branch of javascript-testapp. This change is required because now testapp has to run an additional command to build the bundles which was not the case earlier. This testapp PR needs to be merged before merging this SDK branch.
https://github.com/optimizely/javascript-testapp/pull/83

Test plan

All unit tests and Full stack compatibility suite tests pass.

# Conflicts: # packages/optimizely-sdk/lib/index.browser.js # packages/optimizely-sdk/lib/index.node.js # packages/optimizely-sdk/lib/index.react_native.js

packages/optimizely-sdk/karma.base.conf.js

mjc1283 · 2020-04-08T22:21:55Z

packages/optimizely-sdk/lib/index.browser.tests.js

@@ -57,7 +55,6 @@ describe('javascript-sdk', function() {
        sinon.stub(configValidator, 'validate');

        xhr = sinon.useFakeXMLHttpRequest();
-        global.XMLHttpRequest = xhr;


Is this line not necessary?

It looked redundant and removing it did not affect any tests.

packages/optimizely-sdk/rollup.config.js

mjc1283 · 2020-04-08T22:52:08Z

packages/optimizely-sdk/package.json

    "istanbul": "^0.4.5",
    "json-loader": "^0.5.4",
    "karma": "^4.4.1",
    "karma-browserstack-launcher": "^1.5.1",
    "karma-chai": "^0.1.0",
-    "karma-chrome-launcher": "^2.1.1",


Why is this removed? Is this used for running umd or xbrowser tests locally?

I tried removing modules that are not being used. This is in the devDependencies but is not being used in karma file. It looks like somebody added it to try karma tests locally.

If this is necessary to run it locally, let's leave it in, otherwise OK.

mjc1283 · 2020-04-08T22:52:19Z

packages/optimizely-sdk/package.json

    "karma-mocha": "^1.3.0",
-    "karma-sinon": "^1.0.5",


Why is this removed?

I tried removing modules which were not being used by removing and running tests. Removing this one did not fail any tests so looks like its no longer required

mjc1283 · 2020-04-09T00:38:12Z

packages/optimizely-sdk/package.json

    "istanbul": "^0.4.5",
    "json-loader": "^0.5.4",
    "karma": "^4.4.1",
    "karma-browserstack-launcher": "^1.5.1",
    "karma-chai": "^0.1.0",
-    "karma-chrome-launcher": "^2.1.1",


If this is necessary to run it locally, let's leave it in, otherwise OK.

mjc1283 · 2020-04-09T00:59:24Z

packages/optimizely-sdk/lib/index.browser.js

+  logging: loggerPlugin,
+  errorHandler: defaultErrorHandler,
+  eventDispatcher: defaultEventDispatcher,
+  enums,


Oops, I missed this. @zashraf1985 don't use the property shorthand here. This is not ES5-compatible. Needs to be enums: enums.

This is Done

* v4: test suite doesn't pass cleanly prepare already runs build v4.0.0-github.0 Ignore package tarballs Resume building non-minified ES entrypoint revert to master Expose an ESM-FULL bundle which includes optimizely deps Standardize ESM bundle Generate both min and non-min esm output exports option for es output is redundant. auto is best Use rollup --config- options for specifying bundles Build-all by default build various bundles using --config-* param chore: Prepare for 4.0.0 release (optimizely#468) fix: Removed React Native client engine temporarily (optimizely#466) chore: Prepare for 4.0.0-rc.2 release (optimizely#465) chore: Add source maps to build output (optimizely#464) fix(project config): Don't mutate the datafile object (optimizely#462) chore: Prepare for 4.0.0-rc.1 release (optimizely#461) chore: Prepare for js-sdk-datafile-manager 0.5.0 release (optimizely#460) chore(datafile manager): Remove react native datafile manager entry from package.json (optimizely#459) fix(datafile manager): Node datafile requests use gzip,deflate compression (optimizely#456) docs(datafile manager): Removed manual dependency installation requirement from docs (optimizely#457) chore(datafile manager): Added async storage as dev dependency (optimizely#455) refactor: Added a separate bundle for Json Schema Validator (optimizely#454) refactor: Convert lib/utils to ES module (Part 2/2) (optimizely#452) refactor: Convert lib/core to ES module (Part 2/2) (optimizely#450) chore(datafile manager): Update years in header comments (optimizely#453) refactor: Convert lib/core to ES module (Part 1/2) (optimizely#449) refactor: Convert lib/utils to ES module (Part 1/2) (optimizely#451) refactor: Convert lib/optimizely/* and lib/tests/* to ES module (optimizely#448) refactor: Convert lib/plugins from CJS to ES module (optimizely#427) chore: Update js-sdk-utils to 0.2.0 (optimizely#447) refactor: Convert entry points to es module and create minified bundles (optimizely#445) chore: Update CHANGELOG.MD for jsonSchemaValidator change (optimizely#443) Change functionality of JSON schema validation by removing skipJSONValidation entirely. Now a user needs to import jsonSchemaValidator from @optimizely/optimizely-sdk/lib/utils/json_schema_validator and pass it to createInstance when validation is desired. (optimizely#442) chore(datafile manager): Lint and formatting fixes for datafile manager tests (optimizely#441) [OASIS-6102]: changed functionality of the JSON schema validator module (optimizely#438) chore(datafile manager): Fix ESLint warnings & errors, apply Prettier formatting (optimizely#440) fix (datafile-manager): Fix boolean types, remove StaticDatafileManager, update READMe, add eslint + prettier (optimizely#436)

Cleaned up other PR by fayyaz

0ac6a76

fayyazarshad added the WIP label Apr 7, 2020

fayyazarshad self-assigned this Apr 7, 2020

finalized the code

b89eed7

fayyazarshad force-pushed the zeeshan/esmodule-conversion-entrypoints branch from 21b4434 to b89eed7 Compare April 7, 2020 14:37

fayyazarshad closed this Apr 7, 2020

fayyazarshad reopened this Apr 7, 2020

fayyazarshad force-pushed the zeeshan/esmodule-conversion-entrypoints branch from fee016d to b89eed7 Compare April 7, 2020 19:41

zashraf1985 added 2 commits April 7, 2020 19:09

added buble-loader to transpile es6 code for karma

5e07a49

did some cleanup

f3613ec

zashraf1985 changed the title ~~Converting entry points to es module~~ refactor: Converted entry points to es module and created minified bundles Apr 8, 2020

zashraf1985 changed the title ~~refactor: Converted entry points to es module and created minified bundles~~ refactor: Convert entry points to es module and create minified bundles Apr 8, 2020

fayyazarshad mentioned this pull request Apr 8, 2020

refactor: Convert lib/plugins from CJS to ES module #427

Merged

zashraf1985 added 6 commits April 8, 2020 12:22

cleaned up index.browser.js

cb49cc5

cleaned up node entry

4f520cc

cleaned up react native entry

c6b3b62

cleaned up test files

5cea14d

Merge branch 'master' into zeeshan/esmodule-conversion-entrypoints

32972e1

# Conflicts: # packages/optimizely-sdk/lib/index.browser.js # packages/optimizely-sdk/lib/index.node.js # packages/optimizely-sdk/lib/index.react_native.js

cleaned up rollup and package json

a6ffcbc

zashraf1985 marked this pull request as ready for review April 8, 2020 22:02

zashraf1985 requested a review from a team as a code owner April 8, 2020 22:02

zashraf1985 removed the WIP label Apr 8, 2020

zashraf1985 unassigned fayyazarshad Apr 8, 2020

mjc1283 suggested changes Apr 8, 2020

View reviewed changes

optimizely-codereview-bot assigned fayyazarshad Apr 8, 2020

zashraf1985 and others added 3 commits April 8, 2020 17:12

incorporated review feedback

03d5fee

Merge branch 'master' into zeeshan/esmodule-conversion-entrypoints

ba997a1

removed unused dependencies

69c2c7e

zashraf1985 unassigned fayyazarshad Apr 9, 2020

optimizely-codereview-bot requested a review from mjc1283 April 9, 2020 00:31

optimizely-codereview-bot requested a review from zashraf1985 April 9, 2020 00:31

mjc1283 approved these changes Apr 9, 2020

View reviewed changes

zashraf1985 added 2 commits April 8, 2020 17:48

Added karma-chrome-launcher again to enable local testing

363814c

removed hardcoded testapp branch from travis

7db4f75

mjc1283 suggested changes Apr 9, 2020

View reviewed changes

optimizely-codereview-bot assigned fayyazarshad Apr 9, 2020

changed es6 property shorthand to its full form to support es5

b51fe4c

mjc1283 approved these changes Apr 9, 2020

View reviewed changes

mjc1283 merged commit 8fe8c99 into master Apr 9, 2020

mjc1283 deleted the zeeshan/esmodule-conversion-entrypoints branch April 9, 2020 01:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Convert entry points to es module and create minified bundles #445

refactor: Convert entry points to es module and create minified bundles #445

fayyazarshad commented Apr 7, 2020 •

edited by zashraf1985

Loading

mjc1283 Apr 8, 2020

zashraf1985 Apr 8, 2020

mjc1283 Apr 8, 2020

zashraf1985 Apr 8, 2020

mjc1283 Apr 9, 2020

mjc1283 Apr 8, 2020

zashraf1985 Apr 8, 2020

mjc1283 Apr 9, 2020

mjc1283 Apr 9, 2020 •

edited

Loading

zashraf1985 Apr 9, 2020

refactor: Convert entry points to es module and create minified bundles #445

refactor: Convert entry points to es module and create minified bundles #445

Conversation

fayyazarshad commented Apr 7, 2020 • edited by zashraf1985 Loading

Summary

Why Webpack for Karma?

Note about Testapp

Test plan

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjc1283 Apr 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fayyazarshad commented Apr 7, 2020 •

edited by zashraf1985

Loading

mjc1283 Apr 9, 2020 •

edited

Loading