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.

Sign up for GitHub

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

Jump to bottom

feat(gatsby): support node: protocol when bundling engines #36506

Merged
merged 4 commits into from
Sep 1, 2022
Merged

feat(gatsby): support node: protocol when bundling engines #36506

merged 4 commits into from
Sep 1, 2022

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Aug 31, 2022

Description

With introduction of node: protocol/imports ( https://nodejs.org/api/esm.html#node-imports ) packages begun to use it and this PR adds bundling support for those. Without it we are getting problems like ones described in webpack/webpack#13290

Related Issues

Fixes #36500

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Aug 31, 2022
@pieh pieh mentioned this pull request Aug 31, 2022
Closed
2 tasks
@pieh pieh added topic: render-mode Related to Gatsby's different rendering modes and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Aug 31, 2022
imjoshin
imjoshin approved these changes Aug 31, 2022
View reviewed changes
Copy link
Contributor

@imjoshin imjoshin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me minus the one test change

e2e-tests/production-runtime/cypress/integration/ssr.js Outdated Show resolved Hide resolved
imjoshin
imjoshin suggested changes Aug 31, 2022
View reviewed changes
Copy link
Contributor

@imjoshin imjoshin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol clicked the wrong button

@pieh @imjoshin
Update e2e-tests/production-runtime/cypress/integration/ssr.js
edce092 
Co-authored-by: Josh Johnson <jcjohnson77@gmail.com>
imjoshin
imjoshin previously approved these changes Aug 31, 2022
View reviewed changes
Copy link
Contributor

@imjoshin imjoshin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 :shipit: 🔥

@pieh
Copy link
Contributor Author

pieh commented Aug 31, 2022

Ugh, the node version we use for e2e in circleci doesn't support node: protocol, so build fails even before compiling / validating engines. Will think on how to address that (without just removing the test heh)

@pieh
Copy link
Contributor Author

pieh commented Sep 1, 2022

I did add hacky polyfill to e2e/prod that allows importing node: (we do execute that code during the build so that has to work to test if bundling it works as well), now it should be ready

@pieh pieh merged commit d6293e3 into master Sep 1, 2022
@pieh pieh deleted the fix/engines-support-node-protocol branch September 1, 2022 13:38
@chitoku-k chitoku-k mentioned this pull request Sep 3, 2023
Merged
@ChristianMurphy ChristianMurphy mentioned this pull request Sep 29, 2023
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marvinjude marvinjude marvinjude approved these changes

@imjoshin imjoshin imjoshin left review comments

Assignees
No one assigned
Labels
topic: render-mode Related to Gatsby's different rendering modes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Builds fail: "Built Rendering Engines failed validation failed validation."
3 participants
@pieh @imjoshin @marvinjude