-
Notifications
You must be signed in to change notification settings - Fork 1k
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 @redwoodjs/web to ESM/CJS dual package #10868
Conversation
Rename files without JSX files to just ts
…ckage-cjsonly-new-build * 'main' of github.com:redwoodjs/redwood: (66 commits) chore(testing dbAuth): Add "yes" prompt testing for WebAuthn (redwoodjs#10823) chore(testing dbAuth): Mock console.log to silence test output (redwoodjs#10821) chore(testing dbAuth): Remove duplicated tests (redwoodjs#10822) chore(testing dbAuth): Remove outdated code (redwoodjs#10820) chore(testing dbAuth): Consistent naming and fix blue squiggles (redwoodjs#10819) chore(testing dbAuth): provide mock implementations (redwoodjs#10818) feat(dbAuth): Only suggest dbAuth setup if needed (redwoodjs#10793) chore(dbAuth): Fix test for webauthn prompt in `g dbAuth` (redwoodjs#10814) fix(dbAuth): Print the correct "post message" after generation (redwoodjs#10813) fix(dbauth): Fix spacing issue in task titles (redwoodjs#10811) deps(docs): update braces package (redwoodjs#10809) chore(deps): bump braces from 3.0.2 to 3.0.3 in /tasks/check (redwoodjs#10808) chore(deps): update yarn to v4.3.0 (redwoodjs#10801) chore(deps): update dependency tsx to v4.15.2 (redwoodjs#10800) chore(deps): update dependency @clerk/types to v3.65.2 (redwoodjs#10795) chore(deps): update dependency firebase to v10.12.2 (redwoodjs#10802) chore(deps): update dependency rollup to v4.18.0 (redwoodjs#10799) chore(deps): update dependency jsdom to v24.1.0 (redwoodjs#10798) chore(deps): update dependency @types/vscode to v1.90.0 (redwoodjs#10796) chore(deps): update dependency @clerk/clerk-react to v4.32.2 (redwoodjs#10794) ...
+ Still use cjs bins + Build esm + Update yarn.lock so new bins are picked up
…in ESM packages 🤷
…th being transformed?
Nearly there, apart from cleanup only thing not seemingly working is the Waterfall blog post in storybook-webpack. The success component gets this error (doesn't render the first level success either). Could be a double package hazard?
Manual testing on storybook-vite, and on dev shows that it's actually ok. I'll keep investigating a bit today, but if we're removing storybook-webpack, may not be worth the effort. |
…e/p2-web-esm * 'chore/p2-web-esm' of github.com:dac09/redwood: fix(deps): update graphqlcodegenerator monorepo (redwoodjs#10856)
Ready to start reviewing @Josh-Walker-GM - I still have a bit of cleanup left, but should be good to start looking through :) Thanksssss |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking through everything there isn't anything that sticks out as too unusual. We've talked through some of the changes here and they make sense.
Thanks for documenting the apollo client import cjs/typescript issue as that was something that I hadn't seen before and was a bit different.
It all looks fine and if CI is happy then I too am happy. As you said we will only get better at understanding all this and can tidy up bits and pieces as we do so.
…-esm * 'main' of github.com:redwoodjs/redwood: fix(deps): update dependency nodemailer to v6.9.14 (redwoodjs#10887)
This PR changes redwoodjs/web to a cjs/esm dual package!
There's a lot of hackery to get it all working, but some key unusual changes to make it work:
1. Force importing from cjs files of apollo
We do this for prerender primarily, because using the normal imports ends up failing here. Apollo team will be updating their package so that direct cjs imports will have types too (and we can remove ts-expect-error).
When they release Apollo 4.x - they will introduce the exports field, and we will need to remove the CJS imports.
2. Completely disables HelmetProvider in SSR/RSC
Helmet is not actually used in the SSR+ builds, it uses the internal version -
PortalHead
- to achieve similar results. As before, some features like title templates aren't supported here.I had to do this because I could not get react-helmet-async to reliably import in all conditions - it either breaks one or the other between legacy and ESM builds (SSR/RSC)
3. I had to alias
@redwoodjs/web/apollo
in storybook-webpackI don't understand exactly why, but importing from the CJS version (which webpack correctly picks) does not work in storybook for the Cell success state. It is very likely a dual package hazard, but I can't quite connect the dots as to why changing the import to ESM fixes it.
4. I left the bins as CJS
Since we're building both, I elected to keep it simpler leaving the bins as before
5. Removing babel transforms means we need to import React in all components
We had a babel plugin do autoimports for React, without it we need to make sure all files containing JSX has an import to React.
Other insights useful for anyone else trying to do a dual package conversion:
Red squiggles on existing projects - "cannot import ESM from CJS module"
These don't happen anymore, because we need to generate different types for CJS. See web/build.mts.
Pay attention to the commits
I made a real effort to commit carefully so you can see what changes maybe required to get stuff working. I'll write this up too.
Tasks:
I will do this in a follow up PR