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

chore(esm/cjs): Make the rwjs/vite package dual #10934

Merged
merged 45 commits into from
Jul 16, 2024

Conversation

dac09
Copy link
Collaborator

@dac09 dac09 commented Jul 10, 2024

As title

Outstanding:

  • cleanup
  • understand what's going on with the babel imports, and vitest.
  • generate CJS types
  • add explanatory comments on the fun stuff I'm doing with bundling react-server-dom-webpack
    - [ ] Convert supabase middleware to dual package too, like dbAuth.
    I had to convert dbAuth middleware, because otherwise the RSC builds fail with "default export not found", when trying to import MiddlewareResponse.

I took this out of this PR. I'm going to do this separately, as its not a blocker anymore.

@dac09 dac09 added this to the RSC milestone Jul 10, 2024
@dac09 dac09 added the release:chore This PR is a chore (means nothing for users) label Jul 10, 2024
dac09 and others added 2 commits July 12, 2024 14:09
…ore/vitepkg-dual

* 'chore/vitepkg-dual' of github.com:dac09/redwood:
  fix(docs): Fix introduction link (redwoodjs#10938)
  fix(cli-helpers): Fix CJS support (redwoodjs#10936)
  chore(cli-helpers): Formatting
  chore(cli-helpers): loadEnvFiles cleanup (redwoodjs#10935)
  feat(cli-helpers): Add loadEnvFiles (redwoodjs#10931)
@dac09
Copy link
Collaborator Author

dac09 commented Jul 12, 2024

STUCK - problem 1:

Solved it by generating index.cts. Expand for old problem description

I can't generate CJS types for the vite package, because it complains about:
a) Not being able to resolve imports that use the pkgJson.exports field
b) Babel import workarounds I had to do

The problem is that for us to build to ESM, and use exports properly, we have to set the following properties in the tsconfig.json:

    "module": "NodeNext", // or     "module": "EsNext",
    "moduleResolution": "NodeNext"

which means we change the import statements (to have .js, and use exports fields).

BUT to generate the CJS types, I need to set in the tsconfig.types-cjs.json I need to se:

    "module": "commonjs",
    "moduleResolution": "node", // cannot set NodeNext when module is commonjs

which doesn't compile in TSC due to the reasons above (a) and (b).... 👎

GitHub Actions added 3 commits July 12, 2024 15:46
…g-dual

* 'main' of github.com:redwoodjs/redwood:
  chore(web): Use attw args instead of custom output parsing (redwoodjs#10944)
  chore(web): publint and attw (redwoodjs#10943)
  chore(auth): Fix esm cjs exports and add tooling to verify correctness (redwoodjs#10942)
  chore(middleware): Move middleware out of rwjs/vite -> rwjs/web/middleware (redwoodjs#10917)
  feat(colors): Add more chalk colors. And prepare for chalk upgrade (redwoodjs#10939)
  fix(g directive): Notes formatting (redwoodjs#10940)
@dac09 dac09 marked this pull request as ready for review July 15, 2024 10:39
GitHub Actions added 3 commits July 15, 2024 18:21
…ore/vitepkg-dual

* 'chore/vitepkg-dual' of github.com:dac09/redwood:
  chore(auth): build tweaks and add missing devdep (redwoodjs#10947)
  chore(web): tsconfig.build.json (redwoodjs#10946)
packages/project-config/src/paths.ts Show resolved Hide resolved
packages/vite/package.json Show resolved Hide resolved
packages/vite/package.json Outdated Show resolved Hide resolved
packages/vite/src/devFeServer.ts Show resolved Hide resolved
packages/vite/src/rsc/rscWorkerCommunication.ts Outdated Show resolved Hide resolved
@Tobbe
Copy link
Member

Tobbe commented Jul 15, 2024

Can you please also add attw and publint tests to the vite package, like I've done for auth, web and project-config? Just to make sure they both agree we got the package.json setup properly

…g-dual

* 'main' of github.com:redwoodjs/redwood:
  fix(web): Re-add generating ESM types (redwoodjs#10952)
  chore(project-config): Verify package.json with publint and attw (redwoodjs#10951)
  chore(apollo client): Add types for cjs imports (redwoodjs#10949)
  chore(tsconfig.types-cjs): Specify tsBuildInfoFile location (redwoodjs#10950)
@dac09
Copy link
Collaborator Author

dac09 commented Jul 16, 2024

Can you please also add attw and publint tests to the vite package, like I've done for auth, web and project-config? Just to make sure they both agree we got the package.json setup properly

Added the scripts. Note the failures are for exports that shouldn't sit in the vite package (I think! - all the router stuff) - or are only imported in ESM. I think I prefer to have the default output from attw, rather than filter out messages - what do you think?

I can also add the require ones and generate the .d.ts for them, but in reality we don't want these exports imported in a CJS environment

@dac09 dac09 modified the milestones: RSC, chore Jul 16, 2024
@dac09 dac09 merged commit 08e3d49 into redwoodjs:main Jul 16, 2024
51 checks passed
@dac09 dac09 deleted the chore/vitepkg-dual branch July 16, 2024 10:29
dac09 added a commit to dac09/redwood that referenced this pull request Jul 16, 2024
…-mw-dualpkg

* 'main' of github.com:redwoodjs/redwood:
  RSC: Fix red squiggles in default template (redwoodjs#10955)
  chore(supabase-mw): Convert supabase-mw to dual package (redwoodjs#10954)
  chore(esm/cjs): Make the rwjs/vite package dual (redwoodjs#10934)
  chore(auth): More gracefully handle build failures (redwoodjs#10953)
  fix(web): Re-add generating ESM types (redwoodjs#10952)
  chore(project-config): Verify package.json with publint and attw (redwoodjs#10951)
  chore(apollo client): Add types for cjs imports (redwoodjs#10949)
  chore(tsconfig.types-cjs): Specify tsBuildInfoFile location (redwoodjs#10950)
@Josh-Walker-GM Josh-Walker-GM modified the milestones: chore, v8.0.0 Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:chore This PR is a chore (means nothing for users)
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants