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

feat(es/typescript): Add ts_enum_is_mutable to disable enum inlining #8115

Merged
merged 6 commits into from
Oct 15, 2023

Conversation

dsherret
Copy link
Contributor

@dsherret dsherret commented Oct 13, 2023

Description:

This adds the ability to disable the enum inlining optimization that was recently introduced as the default.

This only updates the Rust API and nothing else.

Related issue:

@magic-akari
Copy link
Member

magic-akari commented Oct 13, 2023

#[serde(default)]
#[deprecated(note = "This value will be always true")]
pub ts_enum_is_readonly: bool,

Something related. never mind it.

@dsherret dsherret marked this pull request as ready for review October 13, 2023 06:30
///
/// Defaults to true.
#[serde(default = "true_by_default")]
pub ts_enum_is_readonly: bool,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@magic-akari oh, I've never noticed

#[serde(default)]
#[deprecated(note = "This value will be always true")]
pub ts_enum_is_readonly: bool,

Should it use it instead? I'm not sure what the API would look like here though.

Copy link
Member

Choose a reason for hiding this comment

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

It's a JavaScript API for end users.
Since we only change the rust side API, this option is irrelevant.

@kdy1 kdy1 changed the title feat(transforms/typescript): add ts_enum_is_readonly feat(es/typescript): Add ts_enum_is_readonly Oct 13, 2023
@kdy1 kdy1 self-assigned this Oct 13, 2023
@kdy1 kdy1 added this to the Planned milestone Oct 13, 2023
kdy1
kdy1 previously approved these changes Oct 13, 2023
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.

Thanks! Someone told me that it's used by some dsl2ts solutions, which sounds like a reasonable usecase.


swc-bump:

  • swc_ecma_transforms_typescript --breaking

@kdy1
Copy link
Member

kdy1 commented Oct 13, 2023

Btw, is this urgent? I don't like updating swc_core when there's a breaking change 🤣

@dsherret
Copy link
Contributor Author

Nope, not urgent. I can wait to upgrade. Maybe #[non_exhaustive] should be added to this struct as well? (I'm going to add some upgrading instructions that we inspect all the changed configuration in swc on each upgrade)

@kdy1
Copy link
Member

kdy1 commented Oct 13, 2023

I requested permission of mdxjs-rs, and the situation must be far better if I have control

@dsherret dsherret changed the title feat(es/typescript): Add ts_enum_is_readonly feat(es/typescript): Add ts_enum_is_mutable Oct 13, 2023
@kdy1 kdy1 changed the title feat(es/typescript): Add ts_enum_is_mutable feat(es/typescript): Add ts_enum_is_mutable to disable enum inlining Oct 15, 2023
@kdy1 kdy1 self-requested a review October 15, 2023 07:36
@kdy1 kdy1 enabled auto-merge (squash) October 15, 2023 07:37
Copy link
Collaborator

@swc-bot swc-bot left a comment

Choose a reason for hiding this comment

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

Automated review comment generated by auto-rebase script

@kdy1 kdy1 merged commit 57b171d into swc-project:main Oct 15, 2023
143 checks passed
@dsherret dsherret deleted the feat_rust_ts_enum_is_readonly branch October 17, 2023 01:51
@dsherret
Copy link
Contributor Author

Thanks @kdy1!

@kdy1 kdy1 modified the milestones: Planned, v1.3.94 Oct 21, 2023
@swc-project swc-project locked as resolved and limited conversation to collaborators Nov 20, 2023
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.

Ability to disable TS enum value inlining?
4 participants