-
-
Notifications
You must be signed in to change notification settings - Fork 89
fix(formatjs): Variable evaluate concatenation #567
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
Conversation
🦋 Changeset detectedLatest commit: 66b8d7d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
kdy1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See https://rustdoc.swc.rs/swc_ecma_minifier/eval/struct.Evaluator.html
Please use it, instead of implemengint it manually.
|
@kdy1 Thanks |
|
I’ve attempted to refactor the evaluation logic by replacing However, it appears that So I leave evaluate_expression_with_visitor here, and I found some code in crates/swc_ecma_minifier/src/compress/pure/strings.rs can concat strings but it's private that I cannot use. |
|
Can you send a PR towards https://github.com/swc-project/swc? |
|
You can patch your crates locally using https://github.com/swc-project/swc/blob/main/scripts/cargo/patch-section.sh See: https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html |
|
OK, I will open a PR to swc. |
|
Waiting for swc-project/swc#11390 I found that plugins/packages/formatjs/transform/src/lib.rs Lines 945 to 974 in 947ff3f
|
|
I think we should move it to the main SWC repository. Nice catch! It seems like I didn't review the PR for fixing the React Compielr support carefully enough. |
**Description:** Improve Evaluator for `@swc/plugin-formatjs` usage. - Support evaluate BinaryExpression. - Add public functions store and resolve_identifier. - Make `eval_as_expr` a public function. **Related issue:** ref: swc-project/plugins#567
|
Do you want me to update swc_core of this repository to reflect Evaluator changes? |
|
Oh… I realized that I forgot to publish the changes. My bad, sorry. |
|
No worries, you can publish it and then upgrade swc_core in this branch. |
kdy1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
kdy1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
A huge refactor to improve variable evaluate for defaultMessage and description.
Fixes #564, Fixes #565, Fixes #566