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

Revert "ref(nextjs): Use virtual file for proxying in proxy loader (#5960)" #6018

Merged

Conversation

lobsterkatie
Copy link
Member

In #5960, we switched from writing temporary files to disk to using virtual files when creating proxy modules in the nextjs SDK's proxy loader. Though it was a good change, it didn't handle certain cases with the potential to crash a user's build process.

Specifically, rollup converts the absolute paths in the template's import * as wrappedFile from <wrapped file> and export * from <wrapped file> into relative paths, but can break them in the process, by doing things like replacing [ and ] with _. Prior to the switch to virtual files, those relative paths only ever had one segment, and so the code compensating for rollup's weirdness only handles basenames. After the switch, the relative paths sometimes have multiple segments, in a pattern which isn't easy to predict. When that happens, the aforementioned compensation code fails and the broken relative paths aren't fixed.

After discussing with @lforst, the author of that PR, we agreed to take a different approach to handling the broken paths. Rather than add a fix on top of that change, I'm choosing to revert it and implement the new approach separately, so that the new work is easier to review and the change is easier to reason about. Otherwise, the fix would include a bunch of undoing of changes, which would muddy the waters. #5960 also contained a few small refactors which don't bear directly on the switch to using virtual files, and reverting allows me to pull those out into a separate PR, again for ease of understanding.

Reverts 2d068e9.

@lobsterkatie lobsterkatie merged commit ef59700 into master Oct 24, 2022
@lobsterkatie lobsterkatie deleted the kmclb-nextjs-revert-5960-use-virtual-rollup-plugin branch October 24, 2022 07:36
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.

None yet

2 participants