Skip to content

fix: handle dynamic import chunks in Webpack 5 #305

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

Merged
merged 2 commits into from
May 14, 2021

Conversation

ascorbic
Copy link
Contributor

@ascorbic ascorbic commented May 14, 2021

When using Webpack 5, Next.JS builds serverless functions with async chunks in a chunks directory as a sibling of the pages and api directories in .next/serverless. Currently we move these functions to the top level in netlify/functions/next_xxxx, meaning that the relative imports of the dynamic chunks in them are incorrect. e.g. with a page like .next/serverless/pages/deeply/nested/file.js the relative import will be something like ../../../chunks/mychunkname.js. Currently this would be moved up to next_deeply_nested_file/nextPage/index.js, with chunks in next_deeply_nested_file/chunks

This PR preserves the directory structure: next_deeply_nested_file/nextPage/pages/deeply/nested/file.js, with chunks in next_deeply_nested_file/nextPage/chunks, so relative imports will work.

In order for our handler to import the nested file, we now also write out an entry point at nextPage/index.js that refers to the next page, e.g. in this example it would contain module.exports = require("./pages/deeply/nested/file.js")

image

@github-actions github-actions bot added the type: bug code to address defects in shipped code label May 14, 2021
@ascorbic ascorbic requested a review from lindsaylevine May 14, 2021 11:41
Copy link

@lindsaylevine lindsaylevine left a comment

Choose a reason for hiding this comment

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

edit: i read the PR body after it makes sense now!!!

@@ -31,7 +31,7 @@ const copyDynamicImportChunks = async (functionPath) => {
// WP5 files are looked for two levels up (../../chunks) in runtime
// This is a hack to make the file one level up i.e. with
// nextPage/nextPage/index.js, the chunk is moved to outer nextPage in a /chunks dir

Choose a reason for hiding this comment

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

can we update this comment to reflect your changes p pls :)

copySync(join(nextDistDir, 'serverless', filePath), nextPageCopyPath, {
overwrite: false,
errorOnExist: true,
})

// Write the import entry point
await writeFile(join(functionDirectory, 'nextPage', 'index.js'), `module.exports = require("./${filePath}")`)

Choose a reason for hiding this comment

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

ah, clever! templating wouldn't work because of the require

@ascorbic ascorbic force-pushed the ll/dynamic-imports-webpack-5 branch from e974077 to 4847929 Compare May 14, 2021 12:01
Copy link

@lindsaylevine lindsaylevine left a comment

Choose a reason for hiding this comment

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

lgtm!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

@ascorbic ascorbic merged commit dae593b into main May 14, 2021
@ascorbic ascorbic deleted the ll/dynamic-imports-webpack-5 branch May 14, 2021 14:12
serhalp pushed a commit that referenced this pull request Apr 5, 2024
…hout --filter (#305)

* test: add simple npm monorepo and deploy from workspace directory without --filter

* test: use empty base and init git repo

* test: use previous cli

* Revert "test: use previous cli"

This reverts commit d5368afb17a1d909f6f9354c10c2da215986cb9a.

* chore: clearer names in create-e2e-fixtures

* chore: upgrade turbo

* test: install monorepo root deps as well with npm

* test: maybe not run turbo in parallel

* test: maybe not run any test in parallel

* chore: more debug information on deployment failures

* revert some debug changes

* Revert "chore: more debug information on deployment failures"

This reverts commit e3cbe529e624b1ba8833892ed8bbf7cf4f57e8f5.

* remove .gitignore from fixture as there is common one

* test: cleanup, remove empty destructuring

* test: install latest cli in smoke test and don't rely on version in runner
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