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

fix: copy vendor over for local dev #2341

Merged
merged 1 commit into from
Oct 19, 2023
Merged

fix: copy vendor over for local dev #2341

merged 1 commit into from
Oct 19, 2023

Conversation

Skn0tt
Copy link
Contributor

@Skn0tt Skn0tt commented Oct 18, 2023

Closes https://linear.app/netlify/issue/FRA-53/netlify-dev-fails-to-run-middleware.

In #2302, we started vendoring Deno dependencies, but we forgot about copying them over for local dev. This PR fixes that.

The main change is a one-liner, but there was a problem within the way esm.sh bundles these two dependencies, and the way we import them via ../vendor:

https://github.com/netlify/next-runtime/blob/5d71bfbba214e88cf59ed335a786806c947a2109/packages/runtime/src/templates/edge/next-dev.js#L1-L2

This was fixed by registering the import_map.json created by Deno for local dev. There was a bug in how that works though, that was only fixed in a recent release of the CLI. This means customers will need to update to that to get local dev working properly.

I've tested this locally with demos/middleware, and it's working well!

@Skn0tt Skn0tt requested a review from a team as a code owner October 18, 2023 15:59
@netlify
Copy link

netlify bot commented Oct 18, 2023

Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo ready!

Name Link
🔨 Latest commit 5d71bfb
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-nx-monorepo-demo/deploys/653000dcb33f43000838b9a0
😎 Deploy Preview https://deploy-preview-2341--netlify-plugin-nextjs-nx-monorepo-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Oct 18, 2023

Deploy Preview for netlify-plugin-nextjs-next-auth-demo ready!

Name Link
🔨 Latest commit 5d71bfb
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-next-auth-demo/deploys/653000dc8f690d00072be69f
😎 Deploy Preview https://deploy-preview-2341--netlify-plugin-nextjs-next-auth-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Oct 18, 2023

Deploy Preview for netlify-plugin-nextjs-demo-all-flags ready!

Name Link
🔨 Latest commit 5d71bfb
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-demo-all-flags/deploys/653000dccb06b900083a11e2
😎 Deploy Preview https://deploy-preview-2341--netlify-plugin-nextjs-demo-all-flags.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Oct 18, 2023

Deploy Preview for netlify-plugin-nextjs-static-root-demo ready!

Name Link
🔨 Latest commit 5d71bfb
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-static-root-demo/deploys/653000dca747220008235bae
😎 Deploy Preview https://deploy-preview-2341--netlify-plugin-nextjs-static-root-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Oct 18, 2023

Deploy Preview for nextjs-plugin-custom-routes-demo ready!

Name Link
🔨 Latest commit 5d71bfb
🔍 Latest deploy log https://app.netlify.com/sites/nextjs-plugin-custom-routes-demo/deploys/653000dc60f8c100089b8edf
😎 Deploy Preview https://deploy-preview-2341--nextjs-plugin-custom-routes-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Oct 18, 2023

Deploy Preview for next-plugin-edge-middleware ready!

Name Link
🔨 Latest commit 5d71bfb
🔍 Latest deploy log https://app.netlify.com/sites/next-plugin-edge-middleware/deploys/653000dc1bce7c0008cf75c9
😎 Deploy Preview https://deploy-preview-2341--next-plugin-edge-middleware.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Oct 18, 2023

Deploy Preview for next-plugin-canary ready!

Name Link
🔨 Latest commit 5d71bfb
🔍 Latest deploy log https://app.netlify.com/sites/next-plugin-canary/deploys/653000dc0c268200083363c2
😎 Deploy Preview https://deploy-preview-2341--next-plugin-canary.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Oct 18, 2023

Deploy Preview for next-i18next-demo ready!

Name Link
🔨 Latest commit 5d71bfb
🔍 Latest deploy log https://app.netlify.com/sites/next-i18next-demo/deploys/653000dcfe24180008c18a35
😎 Deploy Preview https://deploy-preview-2341--next-i18next-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Oct 18, 2023

Deploy Preview for netlify-plugin-nextjs-export-demo ready!

Name Link
🔨 Latest commit 5d71bfb
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-export-demo/deploys/653000dcabe4f500087db717
😎 Deploy Preview https://deploy-preview-2341--netlify-plugin-nextjs-export-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions github-actions bot added the type: bug code to address defects in shipped code label Oct 18, 2023
@Skn0tt Skn0tt self-assigned this Oct 18, 2023
@netlify
Copy link

netlify bot commented Oct 18, 2023

Deploy Preview for netlify-plugin-nextjs-demo ready!

Name Link
🔨 Latest commit 5d71bfb
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-demo/deploys/653000dc7e537a0008388540
😎 Deploy Preview https://deploy-preview-2341--netlify-plugin-nextjs-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Skn0tt
Copy link
Contributor Author

Skn0tt commented Oct 18, 2023

One weird thing is that when I go to http://localhost:8888/shows/100 in the demos/middleware site, Next shows me this:

Screenshot 2023-10-18 at 18 00 16

I can't imagine how this would relate to my changes, so i'll assume this also exists on main. @orinokai is this a known thing?


const newMap = {
scopes: {
"../": importMap.imports,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're moving the imports field generated by Deno into the ../ scope, so it only applies to our Next code and not to other, Netlify-managed code like the bootstrap layer.

@Skn0tt
Copy link
Contributor Author

Skn0tt commented Oct 18, 2023

Once we have the import map in place, we should be able to simplify the vendor.ts file a bit, and can make this change in a lot of our files:

- ../vendor/esm.sh/v91/[email protected]/deno/dist/server/web/spec-extension/request.js
+ https://esm.sh/v91/[email protected]/deno/dist/server/web/spec-extension/request.js

That'll be a small QoL improvement, since it'll make it easier to update versions of the imported files. Gonna do this in a follow-up!

Copy link
Contributor

@MarcL MarcL left a comment

Choose a reason for hiding this comment

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

Thanks for the swift fix @Skn0tt! :shipit:

@Skn0tt Skn0tt merged commit e43fcf2 into main Oct 19, 2023
85 of 88 checks passed
@Skn0tt Skn0tt deleted the fix-vendor-dev branch October 19, 2023 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants