fix: correctly pass resolvedBaseUrl for Next.js 13.2.0+ #26399
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3 flaky tests on run #45231 ↗︎
Details:
|
|||||||||||||||||||||||||||
| Test | Artifacts | |
|---|---|---|
| ... > correctly returns currentRetry |
Output
Video
|
|
| ... > correctly returns currentRetry |
Output
Video
|
|
| ... > correctly returns currentRetry |
Output
Video
|
|
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.
lmiller1990
commented
Apr 3, 2023
| jsConfig: jsConfigResult.jsConfig, | ||
| // Required for Next.js > 13.2.0 to respect tsconfig.compilerOptions.baseUrl | ||
| resolvedBaseUrl: jsConfigResult.resolvedBaseUrl, |
There was a problem hiding this comment.
This is the fix. We could do ...jsConfigResult, but we don't know exactly what else is there (can change at any time), I think this is probably more explicit and maintainable.
mike-plummer
approved these changes
Apr 3, 2023
There was a problem hiding this comment.
Nice work tracking this down ! 🥳
Two thoughts on the changelog entry:
- Should it reference "in Component Testing projects" just to be more explicit?
- Component testing broken with NextJS 13.2.1 #25951 is already referenced in the 12.8.0 changelog - should we add wording around "completed an edge case not covered in our original fix" or something to that effect?
jordanpowell88
approved these changes
Apr 3, 2023
|
@mike-plummer good idea, I updated the changelog. |
astone123
pushed a commit
to kgroat/cypress
that referenced
this pull request
Apr 19, 2023
…6399) * add test project * styles * pass resolvedBaseUrl * make project more minimal * build binaries * changelog * remove css * pass supportedBrowsers to next webpack config * handle case of missing getSupportedBrowsers function * add default function
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Additional details
As of v13.1.7-canary.7, Next.js projects setting
baseUrlto anything other than the default"."would not correctly resolve modules.Diff: vercel/next.js@v13.1.7-canary.6...v13.1.7-canary.7. Lots of files but mostly static content. Here's the exact line that broke us.
infocontainsinfo.resolvedBaseUrl, which we are currently not passing. I updated our nextHandler to include theresolvedBaseUrl, and it works. I added a regression test, too.Steps to test
You could simply revert the fix I made and run the system test - it will fail. Alternatively, you could make a new Next project, change
baseUrltosrc, then update some imports to removesrc- egimport 'src/styles/global.css'would becomeimport 'styles/global.css', which is allowed since you've now got"baseUrl": "src".How has the user experience changed?
It respects
baseUrland works as expected.PR Tasks
cypress-documentation?type definitions?