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

add commonjs support, remove type: module #118

Merged
merged 6 commits into from
May 4, 2022
Merged

Conversation

oshi97
Copy link
Contributor

@oshi97 oshi97 commented May 2, 2022

This PR adds commonjs support by separating the tsconfig into esm and commonjs versions,
updating our api extractor config, and using conditional exports to provide the right entry point.

At one point, this PR added commonjs support by instead building the library with module: commonjs,
and adding a thin esm wrapper to also support es6 syntax.
This was in order to prevent the dual package hazard documented in
https://nodejs.org/api/packages.html#dual-commonjses-module-packages
and
https://github.com/GeoffreyBooth/dual-package-hazard/blob/master/index.mjs
However, from personal testing vite was not able to tree shake our esm-wrapper.

Furthermore, the dual package hazard is primarily a concern when different plugin-like libraries
are written around some central library, for example if people were to write extensions of
answers-headless-react, and these extensions were written in a mix of only esm and only cjs.
Then, users who tried to use both plugins would find themselves with the dual package hazard.
This scenario is very unlikely because we can expect the vast majority of new code to be written in esm.
As a general trend ESM is the new standard for JS libraries, and is also promoted by typescript.

J=SLAP-2074
TEST=manual

test that the dev command works - I can hook up a local answers-headless-react to a local
project, and changes in answers-headless-react will automatically be reflected in my project

test that answers-headless-react works with yext sites SSR in both prod and dev mode
this was tested using npm pack

@oshi97 oshi97 requested a review from a team as a code owner May 2, 2022 14:50
@oshi97 oshi97 changed the title add commonjs support add commonjs support, remove type: module May 2, 2022
@coveralls
Copy link

coveralls commented May 2, 2022

Coverage Status

Coverage remained the same at 69.725% when pulling d0438d6 on dev/cjs-support into 25d21c2 on develop.

@oshi97 oshi97 marked this pull request as draft May 2, 2022 19:50
@oshi97 oshi97 marked this pull request as ready for review May 4, 2022 14:10
@oshi97 oshi97 changed the title add commonjs support, remove type: module add commonjs support using esm wrapper May 4, 2022
@oshi97 oshi97 marked this pull request as draft May 4, 2022 14:19
@oshi97 oshi97 marked this pull request as ready for review May 4, 2022 15:21
@cea2aj
Copy link
Member

cea2aj commented May 4, 2022

nit: can you update the PR title since this doesn't use an esm wrapper?

@oshi97 oshi97 changed the title add commonjs support using esm wrapper add commonjs support, remove type: module May 4, 2022
@oshi97
Copy link
Contributor Author

oshi97 commented May 4, 2022

nit: can you update the PR title since this doesn't use an esm wrapper?

updated!

@oshi97 oshi97 merged commit 568559a into develop May 4, 2022
@oshi97 oshi97 deleted the dev/cjs-support branch May 4, 2022 20:15
@tmeyer2115 tmeyer2115 mentioned this pull request May 11, 2022
tmeyer2115 added a commit that referenced this pull request May 11, 2022
## Version 1.1.0
### Features
- Support was added for Server-Side Rendering of our React Components. (#115)
- A CommonJS distribution of the library was added. (#118)
- Answers Agent information can be passed through to Headless. (#110)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants