-
Notifications
You must be signed in to change notification settings - Fork 36
[FSSDK-12169] Build system update #314
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
base: master
Are you sure you want to change the base?
Conversation
Jest Coverage Report
Show files with reduced coverage 🔻
Test suite run success266 tests passing in 10 suites. Report generated by 🧪jest coverage report action from b04bfed |
rollup.config.mjs
Outdated
| format: 'cjs', | ||
| exports: 'named', | ||
| sourcemap: true, | ||
| globals: { react: 'React' }, |
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.
is this field necessary for non-umd bundles? https://rollupjs.org/configuration-options/#output-globals
rollup.config.mjs
Outdated
| name: 'optimizelyReactSdk', | ||
| exports: 'named', | ||
| sourcemap: true, | ||
| globals: { react: 'React' }, |
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.
previously we had
globals: {
react: 'React',
crypto: 'crypto',
},
does it work without the crypto?
package.json
Outdated
| "directories": { | ||
| "lib": "lib" | ||
| "main": "./dist/react-sdk.min.js", | ||
| "module": "./dist/react-sdk.es.min.js", |
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.
can we get rid of this module entrypoint similar to the JS sdk. Most modern tools should use the exports field anyway
Summary
This pull request modernizes and streamlines the build and CI workflows for the React SDK package. The most significant changes include a complete overhaul of the build system to use a new, unified
rollup.config.js, updates to package outputs and scripts, and improvements to CI workflows for better reliability and speed.Build system modernization and simplification:
scripts/build.js,scripts/winbuild.js, andscripts/config.js) with a new, comprehensiverollup.config.jsthat supports minified/unminified CJS, ESM, and UMD bundles and uses modern Rollup plugin APIs. [1] [2] [3] [4]package.jsonoutputs and exports to use minified bundles by default, added anexportsfield for better ESM/CJS interop, and revised thefilesfield to only include thedistdirectory.package.jsonto clean before building, use Rollup directly, and add a newbuild-umdscript for UMD-only builds.Dependency and tooling updates:
lint-stagedconfiguration to only lint and test files insrc/, and made test runs pass gracefully when no related tests are found.Continuous Integration improvements:
actions/checkoutandactions/setup-node, enabled npm caching, switched fromnpm installtonpm cifor reproducible installs, and standardized on npm as the package manager for coverage reporting. [1] [2] [3] [4]Test plan
Existing test should pass
Issues