-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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(ts/fast-strip): Handle unsupported module
keyword
#10022
Conversation
🦋 Changeset detectedLatest commit: 987bc96 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The implementation is currently in It’s unclear whether we can achieve the same in the @kdy Please let me know your thoughts. Thank you! |
CodSpeed Performance ReportMerging #10022 will degrade performances by 4.49%Comparing Summary
Benchmarks breakdown
|
I think I need to make a AST-breaking change tomorrow, so please feel free to work on it. Please tell me if you prefer to make the change with another PR |
I've found a solution that preserves our current AST structure and avoids introducing any breaking changes. It's straightforward to pass the source text to the Transform pass. However, if we decide to update the AST, we might consider adding a field to specify the Which do you prefer? |
I think the latter one is better, but it would be better if AST-breaking change is a separate PR. If it's the case I can align the date of merging with release date of various tools to reduce impact of the breaking changes. |
b0ada55
to
48bea07
Compare
module
keyword in strip-only modemodule
keyword
This is a bit awkward. We could have simply removed the top-level namespace, but we had to dig into the internal nodes to catch the error. namespace Foo {
namespace Bar {
module Boom { }
}
} |
module
keywordmodule
keyword
Related issue:
module
namespaces #10017