Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix: glob subdirectories in server/chunks to support Turbopack builds#2987

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
pieh merged 6 commits intoopennextjs:mainfromtimneutkens:fix/turbopack
Jul 10, 2025

Conversation

@timneutkens
Copy link
Contributor

@timneutkenstimneutkens commentedJul 5, 2025•
edited
Loading

Description

TLDR: This change makesnext build --turbopack work on Netlify

Was investigating whynext build --turbopack fails when deploying on Netlify:

  • Turns out this glob is targeting.next/server/chunks/* instead of.next/server/chunks/**/* in the standalone dir.
  • With Turbopack the output can have additional subdirectories (could have happened with webpack too if the output folder was ever changed).
  • Upon further investigation it seems that you'd want to collect all of.next/standalone/.next not just.next/* and.next/server/**/*
  • I don't think there should be a limit on.js because that would exclude e.g. sourcemaps which are.js.map files.

For now to verify if there's a specific reason the subdirectories were targeted I've changed the glob to.next/server/**/* in order to see if any tests fail.

Documentation

Tests

TODO: This needs anext build --turbopack app.

You can test this change yourself like so:

  1. TODO

Relevant links (GitHub issues, etc.) or a picture of cute animal

anisurrahman072 reacted with heart emoji
@timneutkens
Copy link
ContributorAuthor

I've added a test fixture and copied thesimple test suite. However I'm unable to run it successfully locally, it seems I may be missing something, let me know if you're able to run it 🙏

@timneutkenstimneutkens marked this pull request as ready for reviewJuly 6, 2025 13:25
@timneutkenstimneutkens requested a review froma team as acode ownerJuly 6, 2025 13:25
@pieh
Copy link
Contributor

pieh commentedJul 8, 2025

Firstly, huge thanks for PR!

  • Turns out this glob is targeting.next/server/chunks/* instead of.next/server/chunks/**/* in the standalone dir.
  • With Turbopack the output can have additional subdirectories (could have happened with webpack too if the output folder was ever changed).

Awesome find, thank you!

  • Upon further investigation it seems that you'd want to collect all of.next/standalone/.next not just.next/* and.next/server/**/*
  • I don't think there should be a limit on.js because that would exclude e.g. sourcemaps which are.js.map files.

Limiting to.js in.next/server/app|pages was done to not bundle.html,.rsc,.meta,.json for content prerendered at build, because we do have custom CacheHandler that doesn't use filesystem for storage so those files would needlesly grow size of produced function without being actually used. Limiting to.js like we do now is probably too restrictive overall and instead probably we should exclude those specific targeted things we do have alternative handling for.

Expanding a bit on selective dirs globbing - it was done to try to limit chances of exceeding function max size. We do generate single function for all routes today (hopefully to be changed with adapters), so it's easier in this setup to go over the edge than with function per route. This makes this change a bit risky for us and instead I'd like to suggest following:

Could we keep this change to only target turbopack builds needs for right now? My understanding just from reading first 2 points is that it could be addressed by adjusting ourserver/chunks/* (andserver/edge-chunks/*?) pattern to allow sub directories.

Then I would follow up with some ~telemetry addition to asses impact of doing the change in this PR almost as-is (just with added exclussion for default filesystem CacheHandler entries) on size of produced lambda to understand if this is generally safe change for us to roll out or would this result in failures (function size too large) for some users.

I'm happy to take over getting this over the finish line if you wish, just let me know what your thoughts are on this.

@pieh
Copy link
Contributor

pieh commentedJul 8, 2025

Limiting to.js in.next/server/app|pages was done to not bundle.html,.rsc,.meta,.json for content prerendered at build, because we do have custom CacheHandler that doesn't use filesystem for storage so those files would needlesly grow size of produced function without being actually used. Limiting to.js like we do now is probably too restrictive overall and instead probably we should exclude those specific targeted things we do have alternative handling for.

I was checking out failing test and seems to me that those failures are actually caused by including those CacheHandler entries in function without making adjustments elsewhere. To be fair the handling is a bit brittle and not resiliant to changes like that, but it was operating under assumption that.html files would not be bundled and ourfs/promises.readFile would throw -

try{
// Attempt to read from the disk
// important to use the `import * as fs from 'fs'` here to not end up in a endless loop
returnawaitofs.readFile(path,options)
}catch(error){
// only try to get .html files from the blob store
if(typeofpath==='string'&&path.endsWith('.html')){
constcacheStore=getMemoizedKeyValueStoreBackedByRegionalBlobStore()
constrelPath=relative(resolve('.next/server/pages'),path)
constfile=awaitcacheStore.get<HtmlBlob>(relPath,'staticHtml.get')
if(file!==null){
if(file.isFullyStaticPage){
constrequestContext=getRequestContext()
// On server initialization Next.js attempt to preload all pages
// which might result in reading .html files from the file system
// for fully static pages. We don't want to capture those cases.
// Note that Next.js does NOT cache read html files so on actual requests
// that those will be served, it will read those AGAIN and then we do
// want to capture fact of reading them.
const{ initializingServer}=initAsyncLocalStorage.getStore()??{}
if(!initializingServer&&requestContext){
requestContext.usedFsReadForNonFallback=true
}
}
returnfile.html
}
}
throwerror
}

I'll attempt to open PR with optimistic change that scales back the globbing part to what seems like turbopack builds specific needs to verify that is indeed what's needed

@timneutkens
Copy link
ContributorAuthor

Only matchingchunks/**/* would work yeah 👍

@piehpieh changed the titleGlob all server files instead of specific subdirectoriesfix: glob subdirectories in server/chunks to support turbopack buildsJul 10, 2025
Copy link
Contributor

@piehpieh left a comment

Choose a reason for hiding this comment

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

Thank You!

@piehpieh merged commite7bf18f intoopennextjs:mainJul 10, 2025
30 checks passed
@timneutkenstimneutkens changed the titlefix: glob subdirectories in server/chunks to support turbopack buildsfix: glob subdirectories in server/chunks to support Turbopack buildsJul 16, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@piehpiehpieh approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@timneutkens@pieh
[8]ページ先頭
©2009-2025 Movatter.jp