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: more CJS compatibility #4

Merged
merged 4 commits into from
Nov 5, 2024
Merged

fix: more CJS compatibility #4

merged 4 commits into from
Nov 5, 2024

Conversation

JasonWeinzierl
Copy link
Owner

@JasonWeinzierl JasonWeinzierl commented Nov 5, 2024

  • Change package.json type to CJS for more compatibility.
  • Replace unbuild with tsup for more default export ergonomics.
    • unbuild was fine as long as we added a patch script to export default in CJS. However that's a bit unexpected in CJS; you would expect to just do const rxjsX = require('eslint-plugin-rxjs-x'); without .default tacked onto the end. tsup lets us build different output for CJS that's more ergonomic for CJS users.
    • Had to remove the assert { type: 'json' } since esbuild doesn't support it combined with named exports.
  • Only support decamelize v5.
    • v6 is ESM-only, which would be fine if users use this plugin in an ESM eslint config. However if they're using a CJS eslint config and they install this package, their package manager will likely pull in decamelize v6 and cause a error trying to import it. Downgrading helps avoid that confusion; advanced users who need v6 can use something like yarn resolutions to override.

unbuild was not setting both module.exports and module.exports.default which hurts compatibility with CJS.  tsup uses esbuild so we'll try it out.  Unfortunately arethetypeswrong complains about Missing export = but it's not a breaking error, just unnecessary.
- decamlize v6 would be fine for ESM, but since we support CJS,
  declaring support for v6 causes users to pull in v6 by default which
  may cause confusion if they don't realize they need to downgrade
  manually. Advanced users can override anyways.
Copy link

github-actions bot commented Nov 5, 2024

LCOV of commit 793a0df during .github/workflows/ci.yml #19

Summary coverage rate:
  lines......: 96.2% (3249 of 3376 lines)
  functions..: 92.9% (196 of 211 functions)
  branches...: 91.2% (613 of 672 branches)

Files changed coverage rate:
                                         |Lines       |Functions  |Branches    
  Filename                               |Rate     Num|Rate    Num|Rate     Num
  =============================================================================
  src/index.ts                           | 100%     97|    -     0|    -      0

- This lets CJS users avoid .default.  The downside is the CJS and ESM types are slightly different: ESM exports as default, while CJS exports as module.exports =.  Hopefully that misalignment doesn't matter...
@JasonWeinzierl JasonWeinzierl merged commit 0b47390 into main Nov 5, 2024
2 checks passed
@JasonWeinzierl JasonWeinzierl deleted the more-cjs branch November 5, 2024 17:55
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.

1 participant