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

Fix CommonJS import compatibility #401

Merged
merged 1 commit into from
Jan 29, 2024
Merged

Fix CommonJS import compatibility #401

merged 1 commit into from
Jan 29, 2024

Conversation

aduth
Copy link
Member

@aduth aduth commented Jan 29, 2024

🛠 Summary of changes

Fixes another regression of #395, where downstream projects can resolve the CommonJS built output, but would treat its code as ES Modules, since the project's top-level package.json includes "type": "module". It's unclear why this wouldn't have been an issue prior to #395.

Example failure: 18F/identity-idp#9992

Error:

TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".ts" for /Code/identity-idp/app/javascript/packages/clipboard-button/clipboard-button-element.spec.ts

I'm not 100% about the solution here since typically a package would only contain a single, root-level package.json. However, I've tested to confirm that it works, and have seen the idea referenced in a few places:

The alternatives could be to use either Babel CLI's (prior to 9.0.0) --out-file-extension .cjs or ESBuild's (after 9.0.0) --out-extension:.js=.cjs to control the file extension output for CommonJS. However, there's not an easy solution to change the source code's path references based on the built output type, aside from using a third-party plugin like babel-plugin-replace-import-extension.

📜 Testing Plan

This is easiest to verify via the pull request mentioned above: 18F/identity-idp#9992

  1. cd identity-idp
  2. git checkout dependabot/npm_and_yarn/18f/identity-design-system-8.1.1
  3. yarn install
  4. yarn mocha app/javascript/packages/clipboard-button/clipboard-button-element.spec.ts
  5. Observe the error mentioned above
  6. Apply fix manually: echo '{"type":"commonjs"}' > node_modules/@18f/identity-design-system/build/cjs/package.json
  7. Re-run yarn mocha app/javascript/packages/clipboard-button/clipboard-button-element.spec.ts
  8. Observe test passes successfully

@aduth
Copy link
Member Author

aduth commented Jan 29, 2024

Visual regression failure is being addressed separately at #402

@aduth aduth merged commit dc0dfae into main Jan 29, 2024
1 of 2 checks passed
@aduth aduth deleted the aduth-require-exports-cjs branch January 29, 2024 20:12
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.

2 participants