Repository navigation
Extend lazy server imports to route handlers and Pages - #99541
Conversation
Tests PassedCommit: 79c9a7d |
c37e58f to
032729b
Compare
032729b to
775fc97
Compare
Enable development-only lazy imports for Node.js route handlers, Pages API routes, and Pages SSR while keeping startup entries eager.
775fc97 to
6632217
Compare
| lazy_dynamic_imports: if supports_lazy_dynamic_imports { | ||
| self.next_config() | ||
| .turbopack_lazy_dynamic_imports_ssr(*self.next_mode().await?) | ||
| } else { | ||
| Vc::cell(false) | ||
| }, | ||
| }; |
There was a problem hiding this comment.
You should not do Vc::cell(false) here
If you disable the feature flag, then you have two possible sources for the Vc containing false:
turbopack_lazy_dynamic_imports_ssr()returns Vc::cell(false)- the literal Vc::cell(false) in the else branch.
And then you'd get two different invocations of get_server_chunking_context_with_client_assets returning an identically behaving but unique chunking context.
There was a problem hiding this comment.
And if you enable the feature flag, you would (intentionally) get two different chunking contexts for instrumentation and middleware.
This would be a problem if you have modules that are chunked by multiple chunking contexts. But I think this should be fine as we have separate middleware and instrumentation module contexts anyway, so there is no actual module overlap anyway.
There was a problem hiding this comment.
Yeah, you are totally right. Sorry I missed this.
| true, // client_assets | ||
| true, // supports_lazy_dynamic_imports |
There was a problem hiding this comment.
Sounds like we want an options struct here, but 2 isn't exactly worth it just yet 🤷
Enable development-only lazy imports for Node.js route handlers, Pages API routes, and Pages SSR while keeping startup entries eager.