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

Register migrateImport to ensure it actually runs #14769

Merged
merged 7 commits into from
Oct 24, 2024

Conversation

adamwathan
Copy link
Member

@adamwathan adamwathan commented Oct 23, 2024

This PR makes sure the migrateImport codemod is properly registered so that it runs as part of the upgrade process.

Test plan

This PR adds a new v3 playground with an upgrade script that you can use to run the upgrade from the local package. When you add a non-prefixed @import to the v3 example, the paths are now properly updated with no errors logged:

Screen.Recording.2024-10-24.at.13.28.03.mov

@adamwathan adamwathan requested a review from a team as a code owner October 23, 2024 19:08
@adamwathan adamwathan force-pushed the fix/register-migrate-import branch from 1bf5560 to 190582d Compare October 23, 2024 19:10
philipp-spiess
philipp-spiess previously approved these changes Oct 23, 2024
@adamwathan
Copy link
Member Author

When testing this manually (when we finally figured out how to do that locally, spoiler this was very hard), we noticed a bunch of warnings still that aren't surfaced when running the tests, because our analyze step runs before we rewrite the imports.

So this isn't actually ready to go yet, we're going to want to resolve that before merging.

@adamwathan adamwathan dismissed philipp-spiess’s stale review October 23, 2024 20:00

It still doesn't work 😄

@philipp-spiess philipp-spiess force-pushed the fix/register-migrate-import branch from aa22971 to fa5549c Compare October 24, 2024 11:32
@philipp-spiess
Copy link
Member

@adamwathan Pushed two changes onto this branch just now:

  1. Fix the error we were seeing where the CSS module resolver we built ourselves to find the CSS root couldn't find relative files without the ./ syntax.
  2. Add a v3 playground with a script that patches some dependencies so that we can properly test the upgrade scripts locally without much hassle. Included a small video in the PR description that showcases how you would use this to verify this fixes the issue.

@philipp-spiess philipp-spiess requested review from philipp-spiess and a team October 24, 2024 11:34
@philipp-spiess philipp-spiess force-pushed the fix/register-migrate-import branch from 81b7124 to 9ece083 Compare October 24, 2024 14:28
@adamwathan adamwathan merged commit 39cfcfa into next Oct 24, 2024
1 check failed
@adamwathan adamwathan deleted the fix/register-migrate-import branch October 24, 2024 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants