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.

Sign up for GitHub

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

Jump to bottom

test_transform should probably use parse_program instead of parse_module #8713

Closed
levi-nz opened this issue Mar 8, 2024 · 2 comments · Fixed by #9264
Closed

test_transform should probably use parse_program instead of parse_module #8713

levi-nz opened this issue Mar 8, 2024 · 2 comments · Fixed by #9264
Labels
enhancement
Milestone
v1.7.33

Comments

@levi-nz
Copy link
Contributor

levi-nz commented Mar 8, 2024

Describe the feature

The problem right now with test_transform is that, because it uses parse_module to parse code internally, visitors that use visit_script/visit_mut_script/fold_script will not work because these functions are never called due to the parsed code being a module (thanks to apply_transform):

swc/crates/swc_ecma_transforms_testing/src/lib.rs

Line 175 in b0c00b9

.parse_module()

To fix this, we could make test_transform use parse_program instead, but this might introduce breaking changes. A Program can be either a Module or Script, but if we make test_transform use parse_program it might break existing unit tests (1st or 3rd party) that use visit_module instead of visit_program (or the other two functions). Some of the code related to this would also need to be changed, but that doesn't seem too big of an issue.

Overall, making test_transform would make people's lives a bit easier when making visitors that use specific functions like visit_script if they know they will never need to use the program or module functions, but I'm curious what the maintainers think about this as I'm not sure if I'm right.

Babel plugin or link to the feature description

No response

Additional context

No response
@levi-nz
Copy link
Contributor Author

levi-nz commented Mar 8, 2024

Also just came to thought, I believe visit_program (and others) will also not work in unit tests because a Module is not a Program, as far as I know. So to my understanding right now, only visit_module/visit_mut_module/fold_module can be called from test_transform, while visit_program and visit_script can never be called, due to the parsed code always being a Module.

@kdy1 kdy1 added this to the Planned milestone Mar 11, 2024
levi-nz added a commit to levi-nz/swc that referenced this issue Jul 16, 2024
@levi-nz
Initial commit
3378a57 
Closes swc-project#8713
@levi-nz levi-nz mentioned this issue Jul 16, 2024
Merged
@levi-nz levi-nz changed the title test_transform should possibly use parse_program instead of parse_module test_transform should probably use parse_program instead of parse_module Jul 18, 2024
levi-nz added a commit to levi-nz/swc that referenced this issue Oct 7, 2024
@levi-nz
Initial commit
22c75b3 
Closes swc-project#8713
@kdy1 kdy1 closed this as completed in #9264 Oct 7, 2024
@kdy1 kdy1 closed this as completed in 166b858 Oct 7, 2024
levi-nz added a commit to levi-nz/swc that referenced this issue Oct 8, 2024
@levi-nz
Initial commit
859a276 
Closes swc-project#8713
@levi-nz levi-nz mentioned this issue Oct 8, 2024
Merged
@kdy1 kdy1 modified the milestones: Planned, v1.7.31 Oct 8, 2024
levi-nz added a commit to levi-nz/swc that referenced this issue Oct 8, 2024
@levi-nz
Initial commit
dc0e8a5 
Closes swc-project#8713
kdy1 pushed a commit that referenced this issue Oct 8, 2024
@levi-nz
feat(es/testing): Parse test code as a Program instead of a Module (
bfea322 
#9623)

**Description:**

This PR addresses the issue described in #8713

**BREAKING CHANGE:**
Will break unit tests that use `fold_module`/`visit_module`/`visit_mut_module` if the visitor is intended to work for both modules and scripts instead of using `fold_program`/`visit_program`/`visit_mut_program`.

When creating visitors, you should use `fold_program`/`visit_program`/`visit_mut_program` if you simply want to visit the top-level node.

When creating tests, the input source code will be parsed using `parse_program` by default. If you need to parse it as a `Module`, you can use `module: Some(true)` in `FixtureTestConfig` (or with `test!(module, ..)`), which will parse it as a `Program::Module`, or `Some(false)` for `Program::Script`. `None` will use `parse_program` (`parse_program` will auto-detect the underlying type).
@kdy1 kdy1 modified the milestones: Planned, v1.7.33 Oct 10, 2024
@swc-bot
Copy link
Collaborator

swc-bot commented Nov 9, 2024

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@swc-project swc-project locked as resolved and limited conversation to collaborators Nov 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
enhancement
Milestone
v1.7.33
Development

Successfully merging a pull request may close this issue.

feat(es/testing): Parse test code as a Program instead of a Module levi-nz/swc
3 participants
@levi-nz @kdy1 @swc-bot