Allow using external packages with a workerd build condition #593
Conversation
Update packages/cloudflare/src/cli/build/utils/workerd.spec.ts Co-authored-by: conico974 <nicodorseuil@yahoo.fr> fixup! feedback fixup! bump aws to 3.5.8
🦋 Changeset detectedLatest commit: 558633d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
| for (const [src, dst] of nodePackages.entries()) { | ||
| try { | ||
| const { exports } = JSON.parse(await fs.readFile(path.join(src, "package.json"), "utf8")); | ||
| const match = src.match(isNodeModuleRegex); |
There was a problem hiding this comment.
We should warn the user if !externalPackages.includes(match.groups.pkg) && hasBuildCondition(exports, "workerd")
There was a problem hiding this comment.
I'm not sure what the best thing to do is here, do you have something in mind?
hasBuildCondition(...) check for traced packages (nodePackages).
When I test with postgres, it would not be traced unless added in serverExternalPackages - if not there it is bundled.
I think the warning would trigger for react-dom and packages in server-external-packages.json
There was a problem hiding this comment.
Ho yeah right.
I think we should still add the warning here, at least for the packages you mentionned (we should probably filter some like react-dom)
One thing we could do is maintain a list of known deps that cause trouble like postgres and look if they're in the user package.json
That's not ideal, but at least it will help a bit.
Another option would be to look at the lockfile, but it will show a lot of false positive (especially for monorepo) and would be complex to maintain (especially for bun and the binary lockfile)
I can't think of anything else at the moment
There was a problem hiding this comment.
FYI, i don't consider this blocking, we could figure this out later if you prefer
46a43d4 to
cf2806c
Compare
|
I have updated the PR so that we can disabled using the workerd condition if it causes troubles. Can I please get a review? |
I am thinking about updating https://opennext.js.org/cloudflare/troubleshooting#my-app-fails-to-build-when-i-import-a-specific-npm-package for now. We should list the packages we know using the workerd condition: Thanks for the review |
Should help with #548
This PR change
bundleServer()to use the "workerd" build condition to allow including code specifically created for workerd.When a package uses the "workerd" build conditions, we copy all the files from the source package - Next would not trace/copy files for the workerd runtime.
Note that the packages have to be declared in the
serverExternalPackagesof the Next config. Otherwise they are bundled during the Next build and not in thenft.jsonfiles as a result OpenNext can not detect them. This behavior is not ideal, happy to hear about any better idea!TODO
@opennext/awsrelease in the package.json/cc @ka-raa-ge @mhart