next.js icon indicating copy to clipboard operation
next.js copied to clipboard

fix: allowDynamicGlobs does not work properly on windows

Open WhiteMinds opened this issue 1 year ago • 1 comments

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 https://github.com/vercel/next.js/pull/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 https://github.com/micromatch/picomatch/pull/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.

WhiteMinds avatar Aug 28 '24 07:08 WhiteMinds

Allow CI Workflow Run

  • [ ] approve CI run for commit: eddf6ce77cb1167d6341465429cf7b54fb8c78ae

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

ijjk avatar Aug 28 '24 07:08 ijjk