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(nextjs): Fix resolution of request async storage module #9259

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

lforst
Copy link
Member

@lforst lforst commented Oct 16, 2023

Fixes #9120
Adresses #9092 (comment)

Previously we failed to properly locate Next.js' request-async-storage module in monorepo setups because we were limiting our search to a subset of paths in webpackConfig.resolve.modules.

Example:

  • webpackConfig.resolve.modules contained ["<absolute_path_to_monorepo>/packages/<project_with_next_js>/node_modules", "<absolute_path_to_monorepo>/packages/<project_with_next_js>"]
  • request-async-storage is located at /<absolute_path_to_monorepo>/node_modules/next/dist/client/components/request-async-storage.external.js
  • Since since request-async-storage was not in webpackConfig.resolve.modules or any subpaths, we marked it as not resolved.

New solution: We look upwards from the webpack resolvable paths in the folder tree using the default node resolving algorithm to look for the nearest next installation. Once found, we try to look for the the request-async-storage modules inside that installation.

Additional considerations

I don't know whether it is enough to check the webpack resolvable locations. Even worse, I don't know whether looking upwards from the webpack resolvable locations is problematic because it might not resemble what webpack is doing.

Adding a dependency

We are using the resolve package because it let's us look upwards from a certain location without using stuff like require.resolve which might trip up webpack.

@lforst lforst mentioned this pull request Oct 16, 2023
3 tasks
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

Let's give it a try!

@lforst lforst merged commit 6ab1068 into develop Oct 16, 2023
85 checks passed
@lforst lforst deleted the lforst-fix-9120 branch October 16, 2023 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants