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

chore: update SWC to latest #216

Closed
wants to merge 3 commits into from
Closed

chore: update SWC to latest #216

wants to merge 3 commits into from

Conversation

alexeagle
Copy link
Member

No description provided.

alexeagle referenced this pull request in swc-project/swc Nov 9, 2023
@alexeagle alexeagle changed the title chore: update latest SWC to 1.3.79 chore: update latest SWC to 1.3.99 Nov 27, 2023
@alexeagle alexeagle force-pushed the mirror_more branch 2 times, most recently from 706e62e to 10eb4ba Compare February 17, 2024 20:03
@alexeagle alexeagle changed the title chore: update latest SWC to 1.3.99 chore: update SWC to latest 1.3.x Feb 17, 2024
@alexeagle
Copy link
Member Author

@jbedard I'm taking another pass at getting this working.

I checked my repro that I filed upstream in swc-project/swc#8265 and it's working correctly now:
alexeagle/swc_8265_repro@f1b6331

however we still get a wrong golden file here when trying to upgrade.

@alexeagle
Copy link
Member Author

@realtimetodie are you still around and interested in this project?

Our guess is that swc-project/swc@7dfdc12#diff-80747e5885ec3975aab81e2ada0a9d27f3319dc0fb45fa38415e19e9aab629eaR224-R232 is only resolving one hop of the symlink that swc sees for the input file, but there are actually two hops under Bazel. So my original repro wasn't sufficient.

It has been a long time so I doubt we could ask the SWC maintainers to do another round-trip fixing this. I think one of us will have to try ourselves.

@jbedard volunteered today to find some time to make the repro more accurately lay out symlinks the same way Bazel does.

@jbedard
Copy link
Member

jbedard commented Feb 20, 2024

I've updated the repro to align more with how bazel lays out the bazel-bin + sandbox directories to reproduce the issue: alexeagle/swc_8265_repro@16bfa92

@alexeagle
Copy link
Member Author

Yay a fix has landed upstream swc-project/swc#8757

@alexeagle alexeagle changed the title chore: update SWC to latest 1.3.x chore: update SWC to latest Apr 4, 2024
@alexeagle
Copy link
Member Author

ugh @jbedard we are still getting

< const _moduleA = require("../../../../../../.cache/bazel/_bazel_runner/4b45c25630ed5c5c6ea70d3e83a5966d/sandbox/linux-sandbox/42/execroot/aspect_rules_swc/examples/paths/src/modules/moduleA");
INFO: From Testing //examples/paths:test_0_test:
---
> const _moduleA = require("./modules/moduleA/index");
FAIL: files "examples/paths/src/index.js" and "examples/paths/expected.js" differ. 

WHY????

@Aghassi
Copy link

Aghassi commented Jul 29, 2024

More for posterity but I'm chiming in here to note this is still an issue as of SWC 1.6.6. We came across it when trying to explore using webpack aliases for something which also requires jsc.paths. So ultimately we had to scrap that plan since it's fraught with sandbox escapes still.

I tried to file a bug with SWC, but I guess I didn't provide enough info and was auto closed swc-project/swc#9344

@alexeagle
Copy link
Member Author

@jbedard and I discussed the simple answer to this is just stop using the sandbox by default. SWC actions don't benefit since they are always one-file-in with no dependencies, so there's no hermeticity check that it's adding for developers. I can green this up now.

@alexeagle
Copy link
Member Author

@Aghassi specifically I think we just recommend

# Workaround SWC issue with symlinks
common --modify_execution_info=SWCCompile=+no-sandbox

I feel like we did this a year ago and then I forgot? But it's simple, no downsides, and we can stop fighting with this.

@alexeagle
Copy link
Member Author

replaced by #272

@alexeagle alexeagle closed this Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants