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

feat(next-core): relay transform plugin #48899

Merged
merged 11 commits into from May 9, 2023
Merged

feat(next-core): relay transform plugin #48899

merged 11 commits into from May 9, 2023

Conversation

kwonoj
Copy link
Contributor

@kwonoj kwonoj commented Apr 26, 2023

What

Part 1 for WEB-848.

This PR implements initial path to enable relay compiler option in Turbopack. PR is based on approach WEB-955, that passing custom plugin transform itself instead of trying to pass down configuration into turbopack. In result, this PR requires counterpart turbopack PR at vercel/turbo#4721.

PR also refactors next-shared's transform bit, as we have grown number of custom transforms.

Unfortunately there are some runtime errors with this transforms, so it is not possible to use relay actually yet. WEB-956 tracks this. (swc-project/plugins#179)

@ijjk ijjk added Font (next/font) Related to Next.js Font Optimization. Turbopack Related to Turbopack with Next.js. created-by: Turbopack team PRs by the turbopack team type: next labels Apr 26, 2023
@kwonoj kwonoj changed the title refactor(next-core): extract custom transforms feat(next-core): relay transform plugin Apr 26, 2023
@kwonoj kwonoj force-pushed the feat-turbopack-relay branch 2 times, most recently from 65d44cc to 9616338 Compare May 1, 2023 16:01
.relay
.as_ref()
.map(|config| {
// [TODO]: There are config mismatches between RelayConfig to swc_relay::Config
Copy link
Member

Choose a reason for hiding this comment

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

Add an linear issue and reference that here

program: &mut Program,
ctx: &TransformContext<'_>,
) -> Option<Program> {
let module_program = unwrap_module_program(program);
Copy link
Member

Choose a reason for hiding this comment

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

One of these should do it:

Suggested change
let module_program = unwrap_module_program(program);
let module_program = std::mem::take(program);
Suggested change
let module_program = unwrap_module_program(program);
let module_program = std::mem::replace(program, Program::dummy());
Suggested change
let module_program = unwrap_module_program(program);
let module_program = std::mem::replace(program, Program::Module(Module::dummy()));

Comment on lines 64 to 71
Some(module_program.fold_with(&mut next_dynamic(
self.is_development,
self.is_server,
self.is_server_components,
NextDynamicMode::Turbo,
FileName::Real(ctx.file_path_str.into()),
self.pages_dir.clone(),
)))
Copy link
Member

Choose a reason for hiding this comment

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

When transform has been change to not return Option

Suggested change
Some(module_program.fold_with(&mut next_dynamic(
self.is_development,
self.is_server,
self.is_server_components,
NextDynamicMode::Turbo,
FileName::Real(ctx.file_path_str.into()),
self.pages_dir.clone(),
)))
*program = module_program.fold_with(&mut next_dynamic(
self.is_development,
self.is_server,
self.is_server_components,
NextDynamicMode::Turbo,
FileName::Real(ctx.file_path_str.into()),
self.pages_dir.clone(),
))

@sokra
Copy link
Member

sokra commented May 2, 2023

needs rebase

@kwonoj kwonoj marked this pull request as ready for review May 9, 2023 06:14
@kwonoj kwonoj force-pushed the feat-turbopack-relay branch 2 times, most recently from 0f8ff7e to 82d437a Compare May 9, 2023 06:45
@ijjk
Copy link
Member

ijjk commented May 9, 2023

Failing test suites

Commit: 27f6f93

pnpm testheadless test/e2e/middleware-dynamic-basepath-matcher/test/index.test.ts

  • Middleware custom matchers basePath > should match query path
Expand output

● Middleware custom matchers basePath › should match query path

expect(received).toBe(expected) // Object.is equality

Expected: "another-page"
Received: "random"

  51 |     await browser.elementById('linkelement').click()
  52 |     const anotherPagePath = await browser.elementById('router-path').text()
> 53 |     expect(anotherPagePath).toBe('another-page')
     |                             ^
  54 |   })
  55 | })
  56 |

  at Object.<anonymous> (e2e/middleware-dynamic-basepath-matcher/test/index.test.ts:53:29)

Read more about building and testing Next.js in contributing.md.

@ijjk ijjk merged commit 71d8064 into canary May 9, 2023
98 checks passed
@ijjk ijjk deleted the feat-turbopack-relay branch May 9, 2023 18:09
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
created-by: Turbopack team PRs by the turbopack team Font (next/font) Related to Next.js Font Optimization. Turbopack Related to Turbopack with Next.js. type: next
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants