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

fix(es/codegen): Implement codegen for static import assertions #3113

Merged
merged 14 commits into from
Dec 28, 2021

Conversation

andreubotella
Copy link
Contributor

@andreubotella andreubotella commented Dec 24, 2021

swc_ecma_codegen:

  • Implement codegen for static import assertions.

swc_ecma_transforms_proposal:

  • Modify the existing import_assertions transform to support assertions in exports.
  • Add tests.

swc:

  • Add a keep_import_assertions option to JscExperimental that will keep import assertions in the compiled output.

node-swc:

  • Add a keepImportAssertions option to the typings for JscConfig.experimental.
  • Also fix a typo in the typings where JscConfig.experimental was spelled as "experimetal".

Description:

Currently import assertions are correctly parsed if the input is TypeScript, or EcmaScript with import_assertions: true, but the codegen step strips them no matter what. This PR adds a keep_import_assertions option to JscConfig to keep them.

BREAKING CHANGE:

None

Related issue (if exists):

Fixes #2802.

swc_ecma_codegen:

    * Implement codegen for static import assertions.

swc_ecma_parser:

    * Make static import assertions a syntax error for ES versions under ES2022

Fixes swc-project#3110.
@CLAassistant
Copy link

CLAassistant commented Dec 24, 2021

CLA assistant check
All committers have signed the CLA.

@RiESAEX
Copy link
Contributor

RiESAEX commented Dec 25, 2021

looks like this will fix #2802 too

@andreubotella andreubotella requested a review from kdy1 December 27, 2021 02:24
@kdy1 kdy1 self-assigned this Dec 27, 2021
crates/swc/src/builder.rs Outdated Show resolved Hide resolved
Andreu Botella added 3 commits December 27, 2021 17:56
use a field in `JscExperimental` rather than a variant in `EsVersion` to
enable import assertions in the output.
@andreubotella andreubotella requested a review from kdy1 December 28, 2021 10:27
Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other changes except this LGTM. Thank you!

I think this can make users think it's an option to enable instead of to keep.

crates/swc/src/config/mod.rs Outdated Show resolved Hide resolved
@kdy1 kdy1 added this to the v1.2.124 milestone Dec 28, 2021
Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@kdy1 kdy1 enabled auto-merge (squash) December 28, 2021 11:04
@kdy1 kdy1 disabled auto-merge December 28, 2021 11:06
@kdy1 kdy1 enabled auto-merge (squash) December 28, 2021 11:07
@andreubotella
Copy link
Contributor Author

Thank you!

Thank you for the review!

@kdy1 kdy1 merged commit c9adf03 into swc-project:main Dec 28, 2021
@andreubotella andreubotella deleted the codegen-import-assertions branch December 28, 2021 11:18
@guybedford
Copy link

This configuration doesn't seem to apply through the .swcrc file correctly currently in the Node.js CLI run, instead only when set manually via -C jsc.experimental.keepImportAssertions=true. Perhaps something was overlooked in this PR?

@kdy1
Copy link
Member

kdy1 commented Jan 15, 2022

@guybedford Ah, my bad. I think it's because there's no code to handle the field in

impl Merge for JscExperimental {
fn merge(&mut self, from: &Self) {
if self.plugins.is_none() {
*self = from.clone();
}
}
}

@swc-project swc-project locked as resolved and limited conversation to collaborators Nov 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Keep import assertions intact in targets that support it
5 participants