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

Skip collecting modules from page dir in the client reference plugin #46020

Merged
merged 2 commits into from
Feb 16, 2023

Conversation

shuding
Copy link
Member

@shuding shuding commented Feb 16, 2023

This is a bug fix as we are currently traversing all modules in the client compiler to collect the reference info. However, this is only relevant to modules in the appClient layer. It fixes some bug where a module is imported by both app/ and pages/ so the same resource path causes a conflict.

Bug

  • Related issues linked using fixes #number
  • Integration tests added
  • Errors have a helpful link attached, see contributing.md

Feature

  • Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
  • Related issues linked using fixes #number
  • e2e tests added
  • Documentation added
  • Telemetry added. In case of a feature if it's used or not.
  • Errors have a helpful link attached, see contributing.md

Documentation / Examples

  • Make sure the linting passes by running pnpm build && pnpm lint
  • The "examples guidelines" are followed from our contributing doc

@ijjk
Copy link
Member

ijjk commented Feb 16, 2023

Failing test suites

Commit: b7cf55f

pnpm testheadless test/e2e/app-dir/app-rendering/rendering.test.ts

  • app dir rendering > SSR only > should run data fetch in parallel
Expand output

● app dir rendering › SSR only › should run data fetch in parallel

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

Expected: true
Received: false

  29 |         // Each part takes 5 seconds so it should be below 10 seconds
  30 |         // Using 7 seconds to ensure external factors causing slight slowness don't fail the tests
> 31 |         expect(duration < 7000).toBe(true)
     |                                 ^
  32 |         expect($('#slow-layout-message').text()).toBe('hello from slow layout')
  33 |         expect($('#slow-page-message').text()).toBe('hello from slow page')
  34 |       })

  at Object.<anonymous> (e2e/app-dir/app-rendering/rendering.test.ts:31:33)

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

@ijjk ijjk merged commit 1668a6a into canary Feb 16, 2023
@ijjk ijjk deleted the shu/8t6u branch February 16, 2023 23:17
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants