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: simplify AST selector to work with compiled code #16

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

psalv
Copy link

@psalv psalv commented Jan 24, 2024

Changes our AST selectors to only look for the defaultMessage property, and then checks if there is a sibling property id with a matching string in the passed translation dictionary.

This loosens how specific we are being in our selector but allows us to target a more robust set of compiled code (which is necessary to support transforming nextjs compiled code).

Because we still require an id property that has a string match in our passed-in string dictionary and a defaultMessage as a part of the same object, this change feels relatively safe (in the scenario where we have this object in our compiled code it's highly likely this is actually a translateable string that we want to swap).

It's important for the translated strings passed to the plugin to be in the format that we need them to end up as in the code, so if we are expecting to inline AST then we should pass in translations that have already been converted to AST.

@psalv psalv requested a review from gaojude January 24, 2024 16:16
@psalv psalv force-pushed the simplify-ast-selector branch from 15c012e to 81cc286 Compare January 24, 2024 16:17
@psalv psalv force-pushed the simplify-ast-selector branch from 81cc286 to da46e84 Compare January 24, 2024 16:20
@psalv psalv merged commit 1c60622 into main Jan 24, 2024
2 checks passed
@psalv psalv deleted the simplify-ast-selector branch January 24, 2024 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants