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

Update edge-dynamic-code-evaluation.mdx #64844

Closed
wants to merge 2 commits into from
Closed

Conversation

paul-vd
Copy link
Contributor

@paul-vd paul-vd commented Apr 21, 2024

Following the MiddlewareConfig typings, this is now named as unstable_allowDynamicGlobs

Following the `MiddlewareConfig` typings, this is now named as `unstable_allowDynamicGlobs`
@paul-vd paul-vd requested review from a team as code owners April 21, 2024 09:31
@paul-vd paul-vd requested review from manovotny and leerob and removed request for a team April 21, 2024 09:31
@ijjk ijjk added the Documentation Related to Next.js' official documentation. label Apr 21, 2024
@ijjk
Copy link
Member

ijjk commented Apr 21, 2024

Allow CI Workflow Run

  • approve CI run for commit: 63acb86

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

Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

We still parse it as unstable_allowDynamic not unstable_allowDynamicGlobs:

if (config.unstable_allowDynamic) {
result.unstable_allowDynamicGlobs = Array.isArray(
config.unstable_allowDynamic
)
? config.unstable_allowDynamic
: [config.unstable_allowDynamic]
for (const glob of result.unstable_allowDynamicGlobs ?? []) {
try {
picomatch(glob)
} catch (err) {
throw new Error(
`${pageFilePath} exported 'config.unstable_allowDynamic' contains invalid pattern '${glob}': ${
(err as Error).message
}`
)
}
}
}

Our tests also use unstable_allowDynamic not unstable_allowDynamicGlobs: https://github.com/vercel/next.js/blob/1e710ab73ce6b6ee896535ef0962956aac2e9649/test/integration/edge-runtime-configurable-guards/test/index.test.js

Is the MiddlewareConfig you linked reachable from usercode or do we document that this is the type you should export your config as?

leerob
leerob previously approved these changes Apr 21, 2024
Copy link
Member

@leerob leerob left a comment

Choose a reason for hiding this comment

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

Edit: nevermind

@leerob leerob dismissed their stale review April 21, 2024 22:44

Missed other comment

@jerome-quere
Copy link

jerome-quere commented Apr 24, 2024

@eps1lon

Is the MiddlewareConfig you linked reachable from usercode or do we document that this is the type you should export your config as?

Not documented but reachable from usercode

import { MiddlewareConfig, NextRequest, NextResponse } from 'next/server'
"next": "14.2.2",

@elartix
Copy link

elartix commented Apr 24, 2024

Hi folks, caught regression issue with unstable_allowDynamic feature in 14.2.x

#51401 (comment)

@leerob
Copy link
Member

leerob commented Dec 16, 2024

@elartix That appears to be solved with #73732.

As mentioned above, the config option is still unstable_allowDynamic – we do not document the type referenced here, which is used internally in Next.js. If we do end up exposing this type in the documentation as something you should use, we can revisit the naming here.

@leerob leerob closed this Dec 16, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation Related to Next.js' official documentation. locked
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants