[bugfix] Fix #3883 lazy load parentLocale in defineLocale, fallback to global if missing #4310
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
marwahaha
changed the title
cmyers
changed the title
|
@ichernev - you should look at this one |
|
@marwahaha No problem I'll take a look at that section and update as necessary. I planned on updating the warnings section for parent-locale also being as that warning was removed some time ago. |
cmyers
added a commit
to cmyers/momentjs.com
that referenced
this pull request
Jan 6, 2018
This adds the description of defining a locale with a parent that hasn't itself been defined or loaded yet which has been available since 2.16.0. This adds the additional behavior when creating a moment of lazy loading the parent locale if it isn't already loaded, or defaulting to the global locale if the parent doesn't exist. See PR (moment/moment#4310) As the next version number isn't determined yet, this should be merged only with the next release that includes the above PR.
This was referenced Jan 6, 2018
|
Submitted momentjs.com PRs for documentation: |
marwahaha
pushed a commit
to moment/momentjs.com
that referenced
this pull request
Mar 2, 2018
* Update Customise parent locale This adds the description of defining a locale with a parent that hasn't itself been defined or loaded yet which has been available since 2.16.0. This adds the additional behavior when creating a moment of lazy loading the parent locale if it isn't already loaded, or defaulting to the global locale if the parent doesn't exist. See PR (moment/moment#4310) As the next version number isn't determined yet, this should be merged only with the next release that includes the above PR. * Update 00-intro.md
This was referenced Mar 3, 2018
This was referenced Mar 14, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3883. It's currently possible to define a locale with a parent that is not yet defined. However, in the case of this bug when attempting to create a moment using a child locale with a parent that hasn't been loaded yet or hasn't been defined causes an exception to be thrown during prepareConfig.
This fix first attempts to load the parent locale during
defineLocaleif it can't be found initially. If in the case that a moment is created or a locale is set with a child who's parent can't be loaded or hasn't been defined,chooseLocalereturns the global locale instead of null. This mitigates the error described in this bug report in the instance the parentLocale can't be located, and follows the same logic when defining a child can't load a parent. I've added a test for this case.As this is an edge case it may not be necessary, but makes for a nicer experience rather than having an unhandled exception thrown. I considered adding a warning in
chooseLocalebut this affected a number of tests.Also, a warning was previously shown to indicate the parentLocale is missing but was removed (assuming due to the ability to define a parent after a child). However, the documentation specifies that it should still show the warning? I'm assuming in this instance the documentation needs updating.
Apologies if these scenarios have already been discussed and addressed.