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

convert to esm #356

Closed
wants to merge 1 commit into from
Closed

convert to esm #356

wants to merge 1 commit into from

Conversation

jtoar
Copy link
Contributor

@jtoar jtoar commented Jan 22, 2024

I'm still working through a few things.

The motivation for tackling @redwoodjs/project-config first was 1) most of our other package's depend on it 2) it's small 3) it would make merging the PR that converts the CLI's Jest tests to Vitest easier because Vitest can't mock require (see redwoodjs/redwood#9863).

I used arethetypeswrong/cli extensively. Right now I'm deeming the "Masquerading as ESM" error it emits acceptable. The code between the ESM and CJS files doesn't differ in functionality, only syntax; shipping two declaration copies of all the declaration files is shipping extra code. Mark Erikson did something similar at first at least here:

Unfortunately, no build tool that I knew of at that time did this by default, and the idea of shipping 99%-duplicate typedefs bothered me. So, I opted to not try to fix this "FalseCJS" issue for our packages (at least for the time being).

(Source: https://blog.isquaredsoftware.com/2023/08/esm-modernization-lessons/#typescript-declarations.)

Note that FalseCJS's fancier name is "Masquerading as CJS". We have "Masquerading as ESM", not CJS. I'm not sure if it's an issue that it's flipped yet.

$ attw ./redwoodjs-project-config.tgz 

@redwoodjs/project-config v6.0.7

Build tools:
- [email protected]
- [email protected]

👺 Import resolved to an ESM type declaration file, but a CommonJS JavaScript file. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseESM.md


┌───────────────────┬─────────────────────────────┐
│                   │ "@redwoodjs/project-config" │
├───────────────────┼─────────────────────────────┤
│ node10            │ 🟢                          │
├───────────────────┼─────────────────────────────┤
│ node16 (from CJS) │ 👺 Masquerading as ESM      │
├───────────────────┼─────────────────────────────┤
│ node16 (from ESM) │ 🟢 (ESM)                    │
├───────────────────┼─────────────────────────────┤
│ bundler           │ 🟢                          │
└───────────────────┴─────────────────────────────┘

Regarding the .js extension in TS code, see redwoodjs/redwood#8456.

@jtoar jtoar force-pushed the ds-esm/project-config branch from ad7ecc9 to 57c32e6 Compare January 22, 2024 17:12
@jtoar jtoar force-pushed the ds-esm/project-config branch from 57c32e6 to 25595ab Compare January 22, 2024 17:21
@jtoar jtoar force-pushed the main branch 2 times, most recently from 4540261 to 5936c21 Compare January 23, 2024 10:42
@jtoar jtoar closed this Jan 23, 2024
@jtoar jtoar deleted the ds-esm/project-config branch January 28, 2024 18:38
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