-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Merge EcmascriptChunkUpdate
s before sending them to the client
#3975
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import { createProxy } from "next/dist/build/webpack/loaders/next-flight-loader/module-proxy"; | ||
|
||
// @ts-expect-error CLIENT_CHUNKS is provided by rust | ||
import client_id, { chunks } from "CLIENT_CHUNKS"; | ||
import client_id, { chunks, chunkListPath } from "CLIENT_CHUNKS"; | ||
|
||
export default createProxy(JSON.stringify([client_id, chunks])); | ||
export default createProxy(JSON.stringify([client_id, chunks, chunkListPath])); | ||
alexkirsz marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,10 @@ | ||
declare global { | ||
function __turbopack_require__(name: any): any; | ||
function __turbopack_load__(path: string): any; | ||
function __turbopack_register_chunk_list__( | ||
chunkListPath: string, | ||
chunksPaths: string[] | ||
): any; | ||
Comment on lines
3
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we merge these and offer a single method:
which loads all the chunks at once and registers the chunk list. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think |
||
function __webpack_require__(name: any): any; | ||
var __webpack_public_path__: string | undefined; | ||
var __DEV_MIDDLEWARE_MATCHERS: any[]; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ use std::io::Write; | |
|
||
use anyhow::{bail, Result}; | ||
use indexmap::indexmap; | ||
use turbo_tasks::{primitives::StringVc, Value}; | ||
use turbo_tasks::{primitives::StringVc, TryJoinIterExt, Value}; | ||
use turbo_tasks_fs::{rope::RopeBuilder, File, FileContent, FileSystemPathVc}; | ||
use turbopack_core::{ | ||
asset::{Asset, AssetContentVc, AssetVc}, | ||
|
@@ -116,17 +116,28 @@ impl Asset for PageLoaderAsset { | |
let this = &*self_vc.await?; | ||
|
||
let chunks = self_vc.get_page_chunks().await?; | ||
|
||
let mut data = Vec::with_capacity(chunks.len()); | ||
for chunk in chunks.iter() { | ||
let path = chunk.path().await?; | ||
data.push(serde_json::Value::String(path.path.clone())); | ||
} | ||
let server_root = this.server_root.await?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The way the page loader asset computed paths was incorrect before, as it wasn't basing them upon the server root path. But this doesn't really have any measurable effect today as we don't support base path. |
||
|
||
let chunk_paths: Vec<_> = chunks | ||
.iter() | ||
.map(|chunk| { | ||
let server_root = server_root.clone(); | ||
async move { | ||
Ok(server_root | ||
.get_path_to(&*chunk.path().await?) | ||
.map(|path| path.to_string())) | ||
} | ||
}) | ||
.try_join() | ||
.await? | ||
.into_iter() | ||
.flatten() | ||
.collect(); | ||
|
||
let content = format!( | ||
"__turbopack_load_page_chunks__({}, {})\n", | ||
stringify_js(&this.pathname.await?), | ||
serde_json::Value::Array(data) | ||
stringify_js(&chunk_paths) | ||
); | ||
|
||
Ok(AssetContentVc::from(File::from(content))) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need this file anymore since we don't have "." imports anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. Instead, we could declare our "PAGE"/"INNER_ASSETS" modules (probably would need to rename them to "@vercel/inner/page" or "@turbopack/inner/page").