Ensures CJS/ESM bundle is compatible with Alpha (sonic), pagesJS (vite), and CRA (webpack) #38
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![semgrep-app[bot]](https://avatars.githubusercontent.com/in/60555?s=60&v=4){kind=small}
yen-tt
changed the title
yen-tt
changed the title
nbramblett
approved these changes
Sep 8, 2023
This was referenced Sep 11, 2023
yen-tt
added a commit
to yext/chat-core
that referenced
this pull request
Sep 11, 2023
this PR migrates the build chain from tsc to rollup, as part of the changes discovered in the investigation from this [PR](yext/chat-ui-react#38): in short, using rollup, the esm path files now include `.mjs` extensions and we no longer need `"type": "module"` in our package.json. J=CLIP-556 TEST=manual see that all unit tests and the the two test repo, node-cjs, and brower-esm, still works as expected published an alpha version `[0.7.0-alpha.23](https://www.npmjs.com/package/@yext/chat-core/v/0.7.0-alpha.23)`, and see that it still works as expected in vite/pagesjs and sonic/alpha. Will publish v0.7.0 once merged
yen-tt
added a commit
to yext/chat-headless
that referenced
this pull request
Sep 11, 2023
this PR migrates the build chain from tsc to rollup, as part of the changes discovered in the investigation from this [PR](yext/chat-ui-react#38): J=CLIP-556 TEST=manual see that all unit tests and the two test site still works as expected published an alpha version `[0.7.0-alpha.38.2](https://www.npmjs.com/package/@yext/chat-headless/v/0.7.0-alpha.38.2)` of headless and alpha version `[0.6.0-alpha.38.3](https://www.npmjs.com/package/@yext/chat-headless-react/v/0.6.0-alpha.38.3)` of headless-react, and see that it still works as expected in vite/pagesjs and sonic/alpha. Will publish v0.7.0 of headless once merged. Then update headless-react dep in a separate PR to publish 0.6.0
nbramblett
approved these changes
Sep 11, 2023
cea2aj
added a commit
to yext/search-core
that referenced
this pull request
Sep 12, 2023
The library exported an esm version, however it didn't work with all esm tools becase some require "type": "module" to be set, or the files to end in ".mjs". By moving over to rollup, we can output the esm files with the ".mjs" extension which makes this library work with vite. This change is based on the approach which Chat decided on: yext/chat-ui-react#38 J=BACK-2527 TEST=manual Manually ran the test site locally and confirmed the requests still fire correctly. I still need to test this with vite.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Investigation:
This PR attempts to fix the following error in alpha/sonic:
This was a result of adding
"type": "module"to our esm bundle for our library to be compatible with vite/pagesJS.Vite, and bundlers such as webpack, follow node's module resolution algorithm. and NodeJS have certain rules on how it recognize files as ES modules (https://nodejs.org/api/packages.html#determining-module-system) -- which include adding
"type": "module"as an option.The fix is to update all of our paths to be explicit in order to include the mjs/js extensions, including (e.g.
./componentsto./components/index.jsand../icons/DualSyncto../icons/DualSync.js).This currently can't be done by typescript compiler (tsc) as it's strictly a nodejs behavior and they don't want to support that (long issue thread here), which is frustrating. So we would have to do it manually or have a script to update import/export paths in the final bundle generated by tsc. This is not ideal and can be error prone.
Solution:
I decided to replace tsc with rollup to bundle our library and append .mjs extension to our final esm bundle. Using
mjsextension instead of.jsalso remove the need to do"type": "module"in our esm package.json, which previously introduce unnecessary caveats and one-off script to inject it into our esm bundle.NOTE: the default interop behavior for rollup is not compatible with alpha/sonic. As such, the rollup config in this PR uses
autoto follow Typescript'sesModuleInteropbehavior in how it transpile named, default, and dynamic imports of external deps (the issue with alpha is related to how react-textarea-autosize was imported into ChatInput)NOTE: updated
tsconfig.jsonto use"jsx": "react"instead of"jsx": "react-jsx"because of the way jsx-runtime is backported to react 16/17 without explicit exports field (React github issue here). We would either need to export two separate bundles for latest and legacy versions in order to continue outputting the JSX syntactic sugar OR we could just outputReact.createElementdirectly. This is common for many React libraries that support older versions like React 16. (e.g. ant design, blueprint UI, evergreen UI)J=CLIP-520
TEST=manual
see that the new build works with: