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

Removing usage of Fibers #181

Merged
merged 4 commits into from
Mar 17, 2023
Merged

Removing usage of Fibers #181

merged 4 commits into from
Mar 17, 2023

Conversation

zendranm
Copy link
Member

@zendranm zendranm commented Mar 7, 2023

This PR removes the only usage of Fibers by removing the custom get function used to override native Meteor.EnvironmentVariable's get method. The behavior of the runWithLocale and getLocale functions after removing the override is the same.

@zendranm zendranm marked this pull request as ready for review March 9, 2023 08:26
Copy link
Contributor

@radekmie radekmie left a comment

Choose a reason for hiding this comment

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

So that'd mean the sameLocaleOnServerConnection option is no longer working? If so, we should make it explicit.

Alternatively, we could still override the get but instead of checking the Fiber, just call the original get and fall back to _getConnectionLocale. It needs testing, though.

@zendranm
Copy link
Member Author

@radekmie You're right, removing the entire override breaks the sameLocaleOnServerConnection logic. I removed only the Fibers part of the override and tested it - works the same as it used to.

@zendranm zendranm self-assigned this Mar 16, 2023
@zendranm zendranm linked an issue Mar 16, 2023 that may be closed by this pull request
@zendranm zendranm merged commit ec23cae into master Mar 17, 2023
@zendranm zendranm deleted the remove-fibers branch March 17, 2023 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Remove usage of Fibers
3 participants