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: allowDynamicGlobs does not work properly on windows #69402

Open
wants to merge 1 commit into
base: canary
Choose a base branch
from

Conversation

WhiteMinds
Copy link

fixes #51401

What?

This will resolve the issue where unstable_allowDynamic in 14.2.x does not meet expectations on Windows most of the time.

Why?

This is because #60699 changed the implementation of isDynamicCodeEvaluationAllowed from micromatch.isMatch() to picomatch().

Although the code appears identical, micromatch internally relies on picomatch version 2.3.1, while the modified next.js depends on picomatch version 4.0.2.

During the major version changes, an important modification micromatch/picomatch#73 was made, which sometimes no longer automatically sets opts.windows, causing the path matching regex to be incorrect on Windows.

As shown below:

const picomatch4 = require('picomatch')
const picomatch2 = require('micromatch/node_modules/picomatch')

console.log(picomatch4('/node_modules/.pnpm/**', undefined, true).state.output)
// \/node_modules\/\.pnpm(?:\/(?!\.)(?:(?:(?!(?:^|\/)\.).)*?)|$)
console.log(picomatch2('/node_modules/.pnpm/**', undefined, true).state.output)
// [\\/]node_modules[\\/]\.pnpm(?:[\\/](?!\.)(?:(?:(?!(?:^|[\\/])\.).)*?)|$)

console.log(picomatch4('/node_modules/.pnpm/**', {}, true).state.output)
// [\\/]node_modules[\\/]\.pnpm(?:[\\/](?!\.)(?:(?:(?!(?:^|[\\/])\.).)*?)|$)
console.log(picomatch2('/node_modules/.pnpm/**', {}, true).state.output)
// [\\/]node_modules[\\/]\.pnpm(?:[\\/](?!\.)(?:(?:(?!(?:^|[\\/])\.).)*?)|$)

How?

Alternatively, this library later added a feature to automatically set the windows option when creating a matcher https://github.com/micromatch/picomatch/blob/570df2f8781bc92e5fece2c16d9ff990c4a8d1da/index.js#L10.

So we can either manually pass opts.windows or pass empty opts to trigger the automatic setting.

@ijjk
Copy link
Member

ijjk commented Aug 28, 2024

Allow CI Workflow Run

  • approve CI run for commit: eddf6ce

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants