Skip to content

Conversation

@bgw
Copy link
Member

@bgw bgw commented Nov 13, 2024

This is split out from the PR that introduces the OperationVc type (#70242) to allow for easier review.

Closes PACK-3683

@ijjk ijjk added created-by: Turbopack team PRs by the Turbopack team. tests Turbopack Related to Turbopack with Next.js. labels Nov 13, 2024
Copy link
Member Author

bgw commented Nov 13, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ijjk
Copy link
Member

ijjk commented Nov 13, 2024

Failing test suites

Commit: a998145

pnpm test-dev-turbo test/e2e/conflicting-app-page-error/index.test.ts (turbopack)

  • Conflict between app file and pages file > should show error overlay for /another
  • Conflict between app file and pages file > should show error overlay for /
  • Conflict between app file and pages file > should support hmr with conflicts
  • Conflict between app file and pages file > should not show error overlay for non conflict pages under app or pages dir
  • Conflict between app file and pages file > should error again when there is new conflict
Expand output

● Conflict between app file and pages file › should show error overlay for /another

expect(received).toContain(expected) // indexOf

Expected substring: "App Router and Pages Router both match path:"
Received string:    "Failed to compile"

  52 |       await assertHasRedbox(browser)
  53 |       if (process.env.TURBOPACK) {
> 54 |         expect(await getRedboxDescription(browser)).toContain(
     |                                                     ^
  55 |           'App Router and Pages Router both match path:'
  56 |         )
  57 |       }

  at toContain (e2e/conflicting-app-page-error/index.test.ts:54:53)
  at retry (lib/next-test-utils.ts:806:14)
  at containConflictsError (e2e/conflicting-app-page-error/index.test.ts:51:5)
  at Object.<anonymous> (e2e/conflicting-app-page-error/index.test.ts:74:7)

● Conflict between app file and pages file › should show error overlay for /

expect(received).toContain(expected) // indexOf

Expected substring: "App Router and Pages Router both match path:"
Received string:    "Failed to compile"

  52 |       await assertHasRedbox(browser)
  53 |       if (process.env.TURBOPACK) {
> 54 |         expect(await getRedboxDescription(browser)).toContain(
     |                                                     ^
  55 |           'App Router and Pages Router both match path:'
  56 |         )
  57 |       }

  at toContain (e2e/conflicting-app-page-error/index.test.ts:54:53)
  at retry (lib/next-test-utils.ts:806:14)
  at containConflictsError (e2e/conflicting-app-page-error/index.test.ts:51:5)
  at Object.<anonymous> (e2e/conflicting-app-page-error/index.test.ts:83:7)

● Conflict between app file and pages file › should support hmr with conflicts

Expected no Redbox but found one
header: Build Error
Next.js (15.0.4-canary.6) is outdated (learn more) (Turbopack)

Failed to compile
description: Failed to compile
source: ./
App Router and Pages Router both match path: /
Next.js does not support having both App Router and Pages Router routes matching the same path. Please remove one of the conflicting routes.

   96 |       // Wait for successful recompilation
   97 |       await browser.loadPage(next.url + '/')
>  98 |       await assertNoRedbox(browser)
      |       ^
   99 |       expect(await browser.elementByCss('p').text()).toContain('index - app')
  100 |
  101 |       await browser.loadPage(next.url + '/another')

  at Object.<anonymous> (e2e/conflicting-app-page-error/index.test.ts:98:7)

● Conflict between app file and pages file › should not show error overlay for non conflict pages under app or pages dir

Expected no Redbox but found one
header: Build Error
Next.js (15.0.4-canary.6) is outdated (learn more) (Turbopack)

Failed to compile
description: Failed to compile
source: ./
App Router and Pages Router both match path: /
Next.js does not support having both App Router and Pages Router routes matching the same path. Please remove one of the conflicting routes.

  105 |     it('should not show error overlay for non conflict pages under app or pages dir', async () => {
  106 |       const browser = await next.browser('/non-conflict')
> 107 |       await assertNoRedbox(browser)
      |       ^
  108 |       expect(await getRedboxHeader(browser)).toBeUndefined()
  109 |       expect(await browser.elementByCss('p').text()).toBe('non-conflict app')
  110 |

  at Object.<anonymous> (e2e/conflicting-app-page-error/index.test.ts:107:7)

● Conflict between app file and pages file › should error again when there is new conflict

Expected no Redbox but found one
header: Build Error
Next.js (15.0.4-canary.6) is outdated (learn more) (Turbopack)

Failed to compile
description: Failed to compile
source: ./
App Router and Pages Router both match path: /
Next.js does not support having both App Router and Pages Router routes matching the same path. Please remove one of the conflicting routes.

  117 |     it('should error again when there is new conflict', async () => {
  118 |       const browser = await next.browser('/')
> 119 |       await assertNoRedbox(browser)
      |       ^
  120 |
  121 |       // Re-trigger the conflicted errors
  122 |       await next.renameFile('pages/index2.js', 'pages/index.js')

  at Object.<anonymous> (e2e/conflicting-app-page-error/index.test.ts:119:7)

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

pnpm test-dev test/e2e/app-dir/navigation/navigation.test.ts

  • app dir - navigation > navigating to dynamic params & changing the casing > should load the page correctly
Expand output

● app dir - navigation › navigating to dynamic params & changing the casing › should load the page correctly

expect(received).toContain(expected) // indexOf

Expected substring: "noParam page"
Received string:    "/paramA/paramB
/paramA/noParam"

  923 |
  924 |       await retry(async () => {
> 925 |         expect(await browser.elementByCss('body').text()).toContain(
      |                                                           ^
  926 |           'noParam page'
  927 |         )
  928 |       })

  at toContain (e2e/app-dir/navigation/navigation.test.ts:925:59)
  at retry (lib/next-test-utils.ts:806:14)
  at Object.<anonymous> (e2e/app-dir/navigation/navigation.test.ts:924:7)

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

@bgw
Copy link
Member Author

bgw commented Jan 3, 2025

The OperationVc API has since been overhauled (the ::new() constructor is gone in place of a macro argument) and these change have been done again with the new API across many smaller PRs.

@bgw bgw closed this Jan 3, 2025
bgw added a commit that referenced this pull request Jan 17, 2025
`VcArc` represents values passed through napi to next.js and back. Because these values exit the scope of turbo-tasks, similar to how `State` works, they must be operations, so that they're recomputed/"kept alive" when read.

These changes were extracted from @sokra's original `OperationVc` work (mostly preserved here: #72776), though adapted significantly for the removal of the `OperationVc::new()` constructor.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 17, 2025
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. locked tests Turbopack Related to Turbopack with Next.js.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants