-
Notifications
You must be signed in to change notification settings - Fork 98
fix: compatability for next@16.1.0-canary.19 #3300
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
Conversation
📊 Package size report 0.03%↑
Unchanged files
🤖 This report was automatically generated by pkg-size-action |
820fd16 to
f6c7490
Compare
| // we need to pick the highest version from alternatives and to handle | ||
| // comparison of both range selectors (^) and pinned prerelease version (-rc-<hash>-<date>) | ||
| // we need to use couple of tricks: | ||
| // 1. we do try to parse semver - this only works for pinned versions and will handle prereleases, it will return null for ranges | ||
| // 2. if parsing returns null, we coerce | ||
| // this will allow us to preserve prerelease identifiers for comparisons (as coercing prerelease version strip those) | ||
|
|
||
| const versionToCompare = (parseSemver(selector) ?? coerce(selector))?.format() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not actually solving a particular problem (actually it doesn't solve any I could see), but as I was debugging I failures I did notice we were using react 19 rc instead of stable for our tests against next versions that allowed stable 19. The rc was over a year old and unlikely that would be used by users, so this is mostly to increase confidence in results.
| // due to changes in https://github.com/vercel/next.js/pull/86591 , this global is specific to instance of application and we to clean it up | ||
| // from any previous function invocations that might have run in the same process | ||
| delete globalThis[Symbol.for('next.server.manifests')] | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is integration test setup - only change, doesn't impact deployed apps
da29137 to
00c8d5e
Compare
| if ('${{ github.event_name }}' === 'workflow_dispatch') { | ||
| core.setOutput('matrix', '${{ github.event.inputs.versions }}'); | ||
| } else if ('${{ github.event_name }}' === 'schedule' || versionsToTest === 'all') { | ||
| core.setOutput('matrix', '["latest", "canary", "15.5.9", "14.2.35", "13.5.1"]'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of note - I did bump v14 patch version here AND added v15 as we were not running against it and there will be big chunk of users still using it, so we should make sure we test it
d3f04c2 to
2dc2d3f
Compare
| }, | ||
| "dependencies": { | ||
| "next": "15.1.9", | ||
| "next": "15.1.11", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apparently you should upgrade ;)
| */ | ||
| get outputFileTracingRoot(): string { | ||
| return ( | ||
| // Up until https://github.com/vercel/next.js/pull/86812 we had direct access to computed value of it with following |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was main reason for things not working on next@canary (other than the changed interaction with certain global property in integration tests, that I mentioned in other comment)
The fix is not ideal, but as of now there is no alternative other than using experimental Adapters API to get a hold of fully resolved config, which I rather not do in production
| const promises: Promise<void>[] = [] | ||
|
|
||
| const nodeModulesLocationsInStandalone = new Set<string>() | ||
| const nodeModulesLocations = new Set<{ source: string; destination: string }>() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the adjustments in this function were not related to recent next@canary, but I did want to test my other changes on windows and learnt that this code was not working on it
it's functionally the same as it was, with one adjustments to use destination from cp#filter callback instead of generating it + handling of windows paths to make it work there
Description
This is mostly catching on recent next@canary changes to ensure compatibility for upcoming 16.1 minor release.
In the process of working things out, as I was doing some path related operations and those tend to sometimes cause troubles on windows if not tested I also learned that some of the not-yet released to npm code was not working on windows so I addressed that.
I tried to put more targeted context in code comments and github comments that is specific to each piece I touched
Tests
As we didn't have a way to easily run windows tests on demand I did add "test on windows" label so we could trigger those if we want to ensure future path handling related changes don't break it.
I also updated pinned next version in one of our fixtures AND added most recent v15 version to run on release PRs, as we did not do that when v16 was released, so we were running a little blind here