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

Add env replacer based on VisitMut #9852

Merged
merged 3 commits into from
Jul 17, 2024
Merged

Conversation

yamadapc
Copy link
Contributor

#9828

This is aimed at improving moving SWC usage to visit for all parcel
use-cases as we are facing memory errors due to stack-overflow. Errors
include segmentation faults and bus errors.

This is part of #9828. Adds exhaustive tests for the folder.
Update transformer to use the VisitMut impl.

This replaces the Fold env replacer with a VisitMut implementation.
All tests pass on both old/new versions.

The following behaviours of the fold implementation should be revised in
the future as they might be unwanted:

  • process.browser = ... assignment expressions are replaced with
    process.browser = true, which is not a safe replacement
  • expression replacements const x = ({ x } = process.env); are
    supported but insert an extra trailing object `const x = (x = 'asdf', {});
  • when replace_env replacement is off the transformer still replaces
    several cases that aren't the base declarations case

Overall could simplify this so that less cases are supported, and then
shift it to be a generalised inline variable replacement implementation.

This is aimed at improving moving SWC usage to visit for all parcel
use-cases as we are facing memory errors due to stack-overflow. Errors
include segmentation faults and bus errors.

This is part of #9828. Adds exhaustive tests for the folder.

The next commit will remove the fold implementation and demonstrate
tests pass for the visit implementation as well.

The following behaviours of the fold implementation should be revised in
the future as they likely are unwanted:

* `process.browser = ...` assignment expressions are replaced with
  `process.browser = true`, which is not a safe replacement
* expression replacements `const x = ({ x } = process.env);` are
  supported but insert an extra trailing object `const x = (x = 'asdf', {});
* when `replace_env` replacement is off the transformer still replaces
  several cases that aren't the base declarations case

Overall could simplify this so that less cases are supported, and then
shift it to be a generalised inline variable replacement implementation.

Currently this implementation is quite large to handle things that
aren't necessarily needed or wanted.
This replaces the Fold env replacer with a VisitMut implementation.

All tests pass on this new version with no real change. A case was added
for the `delete process.env.value` behaviour.
@yamadapc yamadapc merged commit e0793ae into v2 Jul 17, 2024
17 checks passed
@yamadapc yamadapc deleted the issue/env-replacer-swc-visit branch July 17, 2024 04:27
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.

3 participants