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

Drop locales & message from resolvedOptions() #63

Merged
merged 2 commits into from
May 6, 2024
Merged

Drop locales & message from resolvedOptions() #63

merged 2 commits into from
May 6, 2024

Conversation

eemeli
Copy link
Member

@eemeli eemeli commented May 6, 2024

Closes #53
Closes #55
Replaces #57

@eemeli eemeli requested a review from ryzokuken May 6, 2024 06:45
Copy link
Member

@ryzokuken ryzokuken left a comment

Choose a reason for hiding this comment

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

LGTM, one comment inline.

Comment on lines +901 to +902
1. Let _locales_ be CreateArrayFromList(_ctx_.[[MessageFormat]].[[RequestedLocales]]).
1. Perform ! CreateDataPropertyOrThrow(_funcCtx_, *"locale"*, _locales_).
Copy link
Member

Choose a reason for hiding this comment

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

Any specific reason for breaking this line up? FWIW I don"t mind either way, just curious.

Copy link
Member Author

Choose a reason for hiding this comment

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

Note that previously we didn"t have a CreateArrayFromList() call here; that was due to the bug noted in #52. So I added it here to also ensure that each time we"re passing in a new array to the called function, which previously would"ve been available for use as a communication channel.

As for splitting up the line, it just got a bit too long for comfort when including the call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants