Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

When initing l10n, cache contents of FTL files, not user locale (#3170) #3199

Merged
merged 2 commits into from
Jul 25, 2017

Conversation

jaredhirsch
Copy link
Member

Fixes #3170 for me locally

@jaredhirsch jaredhirsch force-pushed the 3170-l10n-cleanup branch 2 times, most recently from 7990248 to e91e76e Compare July 24, 2017 20:35
* Also, bail if locales aren't found or can't be loaded.
Copy link
Contributor

@ianb ianb left a comment

Choose a reason for hiding this comment

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

I'm a little concerned about a l10n.init dogpile, but probably it's okay?

});
}).catch(err => {
mozlog.info("l10n-error", {msg: "Error loading FTL files", description: err});
l10n.init().then(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This (l10n.init()) is weird in a handler, but without #2208 I guess it's the best we got.

if (inited) {
return Promise.resolve();
}
inited = true;
exports.userLangs = userLocales;
Copy link
Contributor

Choose a reason for hiding this comment

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

there's a possibility of a dogpile here, if multiple requests come in early. It should sort itself out, but I think it would be safer to do:

let initPromise;
exports.init = function() {
  if (initPromise) {
    return initPromise;
  }
  initPromise = globby(...);
  return initPromise;
};

@jaredhirsch jaredhirsch merged commit 26daf9f into master Jul 25, 2017
@jaredhirsch jaredhirsch deleted the 3170-l10n-cleanup branch July 25, 2017 21:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants