Skip to content

fix(turbopack): Pass resourceQuery to loaders #69703

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.

Sign up for GitHub

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

Merged
merged 14 commits into from
Sep 5, 2024
Merged

fix(turbopack): Pass resourceQuery to loaders #69703

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
61b2493
Add a test
kdy1 Sep 5, 2024
8874251
More work for test
kdy1 Sep 5, 2024
cf375b2
Fix parsing of query string
kdy1 Sep 5, 2024
7226bea
Update test
kdy1 Sep 5, 2024
a956df8
Fix
kdy1 Sep 5, 2024
b904e99
Merge branch 'canary' into kdy1/pack-3228
kdy1 Sep 5, 2024
6010e3a
Only turbopack
kdy1 Sep 5, 2024
408b6a2
query as a separate arg
kdy1 Sep 5, 2024
ebb2418
minimal diff
kdy1 Sep 5, 2024
9f1d012
fmt
kdy1 Sep 5, 2024
03cb3fc
srartsWith
kdy1 Sep 5, 2024
c982bce
fmt
kdy1 Sep 5, 2024
7d5a401
review
kdy1 Sep 5, 2024
722e3ec
Update turbopack/crates/turbopack-node/js/src/transforms/webpack-load…
kdy1 Sep 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions test/e2e/app-dir/turbopack-loader-resource-query/app/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { ReactNode } from 'react'
export default function Root({ children }: { children: ReactNode }) {
return (
<html>
<body>{children}</body>
</html>
)
}
7 changes: 7 additions & 0 deletions test/e2e/app-dir/turbopack-loader-resource-query/app/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// @ts-expect-error -- ignore
import { v } from './test.mdx?test=hi'

export default function Page() {
console.log(v)
return <p>hello world</p>
}
1 change: 1 addition & 0 deletions test/e2e/app-dir/turbopack-loader-resource-query/app/test.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
## Hello World
15 changes: 15 additions & 0 deletions test/e2e/app-dir/turbopack-loader-resource-query/next.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/** @type {import('next').NextConfig} */
const nextConfig = {
experimental: {
turbo: {
rules: {
'*.mdx': {
loaders: ['test-loader.js'],
as: '*.js',
},
},
},
},
}

module.exports = nextConfig
4 changes: 4 additions & 0 deletions test/e2e/app-dir/turbopack-loader-resource-query/node_modules/test-loader.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions test/e2e/app-dir/turbopack-loader-resource-query/turbopack-loader-resource-query.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { nextTestSetup } from 'e2e-utils'
;(process.env.TURBOPACK ? describe : describe.skip)(
'turbopack-loader-resource-query',
() => {
const { next } = nextTestSetup({
files: __dirname,
})

// Recommended for tests that check HTML. Cheerio is a HTML parser that has a jQuery like API.
it('should pass query to loader', async () => {
await next.render$('/')

expect(next.cliOutput).toContain('resource query: ?test=hi')
})
}
)
6 changes: 5 additions & 1 deletion turbopack/crates/turbopack-core/src/ident.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ impl ValueToString for AssetIdent {

let query = self.query.await?;
if !query.is_empty() {
write!(s, "?{}", &*query)?;
if query.starts_with('?') {
write!(s, "{}", &*query)?;
} else {
write!(s, "?{}", &*query)?;
}
Copy link
Member

Choose a reason for hiding this comment

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

query.starts_with('?') should always be true.

This points out a bug somewhere else

}

if let Some(fragment) = &self.fragment {
Expand Down
71 changes: 36 additions & 35 deletions turbopack/crates/turbopack-node/js/src/transforms/webpack-loaders.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,30 @@ import { type StructuredError } from "src/ipc";

export type IpcInfoMessage =
| {
type: "fileDependency";
path: string;
}
type: "fileDependency";
path: string;
}
| {
type: "buildDependency";
path: string;
}
type: "buildDependency";
path: string;
}
| {
type: "dirDependency";
path: string;
glob: string;
}
type: "dirDependency";
path: string;
glob: string;
}
| {
type: "emittedError";
severity: "warning" | "error";
error: StructuredError;
}
type: "emittedError";
severity: "warning" | "error";
error: StructuredError;
}
| {
type: "log";
time: number;
logType: string;
args: any[];
trace?: StackFrame[];
};
type: "log";
time: number;
logType: string;
args: any[];
trace?: StackFrame[];
};

export type IpcRequestMessage = {
type: "resolve";
Expand All @@ -54,9 +54,9 @@ export type IpcRequestMessage = {
type LoaderConfig =
| string
| {
loader: string;
options: { [k: string]: unknown };
};
loader: string;
options: { [k: string]: unknown };
};

let runLoaders: typeof import("loader-runner")["runLoaders"];
try {
Expand Down Expand Up @@ -168,6 +168,7 @@ const transform = (
ipc: Ipc<IpcInfoMessage, IpcRequestMessage>,
content: string,
name: string,
query: string,
loaders: LoaderConfig[]
) => {
return new Promise((resolve, reject) => {
Expand All @@ -180,7 +181,7 @@ const transform = (

runLoaders(
{
resource,
resource: resource + (query ? query.startsWith('?') ? query : `?${query}` : ""),
context: {
_module: {
// For debugging purpose, if someone find context is not full compatible to
Expand Down Expand Up @@ -468,13 +469,13 @@ const transform = (
if (!result.result) return reject(new Error("No result from loaders"));
const [source, map] = result.result;
resolve({
source: Buffer.isBuffer(source) ? {binary: source.toString('base64')} : source,
source: Buffer.isBuffer(source) ? { binary: source.toString('base64') } : source,
map:
typeof map === "string"
? map
: typeof map === "object"
? JSON.stringify(map)
: undefined,
? JSON.stringify(map)
: undefined,
});
}
);
Expand All @@ -494,15 +495,15 @@ function makeErrorEmitter(
error:
error instanceof Error
? {
name: error.name,
message: error.message,
stack: parseStackTrace(error.stack),
}
name: error.name,
message: error.message,
stack: parseStackTrace(error.stack),
}
: {
name: "Error",
message: error,
stack: [],
},
name: "Error",
message: error,
stack: [],
},
});
};
}
2 changes: 2 additions & 0 deletions turbopack/crates/turbopack-node/src/transforms/webpack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,9 @@ impl WebpackLoadersProcessedAsset {
resolve_options_context: Some(transform.resolve_options_context),
args: vec![
Vc::cell(content.into()),
// We need to pass the query string to the loader
Vc::cell(resource_path.to_string().into()),
Vc::cell(this.source.ident().query().await?.to_string().into()),
Vc::cell(json!(*loaders)),
],
additional_invalidation: Completion::immutable(),
Expand Down
Loading