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

Cannot use a locale defined with parent #3883

Closed
vincent-tr opened this issue Apr 4, 2017 · 4 comments · Fixed by #4310, kornicameister/korni#256, JetBrains/ring-ui#157, severest/retrobot#52 or y-yagi/travel_base#463
Labels

Comments

@vincent-tr
Copy link

vincent-tr commented Apr 4, 2017

Description of the Issue and Steps to Reproduce:

const moment = require('moment');

moment.defineLocale('absoluta', {
  parentLocale : 'fr',
  monthsShort  : ['Jan', 'Fev', 'Mar', 'Avr', 'Mai', 'Jui', 'Jul', 'Aou', 'Sep', 'Oct', 'Nov', 'Dec']
});

// unset globally set locale
moment.locale(false);

// uncomment to fix
// moment.localeData('fr');

console.log(moment('08:00:00 01/Avr/17', 'HH:mm:ss DD/MMM/YY', 'absoluta'));

This code is throwing the follwing exception:

TypeError: Cannot read property 'preparse' of null
    at prepareConfig (.../node_modules/moment/moment.js:2518:43)
    at createFromConfig (.../node_modules/moment/moment.js:2497:40)
    at createLocalOrUTC (.../node_modules/moment/moment.js:2584:12)
    at createLocal (.../node_modules/moment/moment.js:2588:12)
    at hooks (.../node_modules/moment/moment.js:16:25)
    at Object.<anonymous> (.../bin/moment-test.js:13:1)
    at Module._compile (module.js:571:32)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)

If I uncomment the line moment.localeData('fr');, it seems to be fixed.

Environment:
Moment 2.18.1
NodeJS 7.7.4 on OSX

@maggiepint
Copy link
Member

The following code just ran totally fine for me in chrome console on momentjs.com:

moment.defineLocale('absoluta', {
  parentLocale : 'fr',
  monthsShort  : ['Jan', 'Fev', 'Mar', 'Avr', 'Mai', 'Jui', 'Jul', 'Aou', 'Sep', 'Oct', 'Nov', 'Dec']
});
moment.locale(false);
moment('08:00:00 01/Avr/17', 'HH:mm:ss DD/MMM/YY', 'absoluta').format('MMM')

The only thing I can think is that you don't have the french locale loaded. If that's the case, parenting to it won't work. If you're using a default node.js configuration though, I can't see why that would happen. We load all locales by default in node.

@vincent-tr
Copy link
Author

I have a "standard" nodejs installation, from brew on osx.
Let me know how to provide more information about that.

@icambron
Copy link
Member

icambron commented May 3, 2017

Yeah, I can reproduce this in Node. It must be that we're failing to lazy load in the parent lang in Node. I don't remember quite how that stuff works, so I'm not sure where the bug and fix would be exactly, though.

@icambron icambron added the Bug label May 3, 2017
cmyers added a commit to cmyers/moment that referenced this issue Oct 19, 2017
cmyers added a commit to cmyers/moment that referenced this issue Oct 19, 2017
@cmyers
Copy link
Contributor

cmyers commented Oct 19, 2017

Currently looking at this as it's similar to #3626 which I've submitted a PR for. defineLocale returns null if the parent locale is not found. A potential fix would be to lazy load the locale if the parent locale can't be found (see referenced commit). However, this would still fail with the same error this bug details if the parent locale doesn't exist.

If the parent locale can't be found, localeFamilies is populated which would be later used to define further locales, but returns null before this occurs. Given this, would some further handling be required if a parent locale can't found/loaded? Currently studying the logic around locale loading, so may revisit with further commits, or if the fix is sufficient I can submit a PR.

cmyers added a commit to cmyers/moment that referenced this issue Nov 14, 2017
cmyers added a commit to cmyers/moment that referenced this issue Nov 18, 2017
cmyers added a commit to cmyers/moment that referenced this issue Nov 18, 2017
cmyers added a commit to cmyers/moment that referenced this issue Nov 18, 2017
cmyers added a commit to cmyers/moment that referenced this issue Nov 18, 2017
cmyers added a commit to cmyers/moment that referenced this issue Nov 18, 2017
cmyers added a commit to cmyers/moment that referenced this issue Nov 18, 2017
cmyers added a commit to cmyers/moment that referenced this issue Nov 18, 2017
cmyers added a commit to cmyers/moment that referenced this issue Nov 18, 2017
cmyers added a commit to cmyers/moment that referenced this issue Nov 18, 2017
cmyers added a commit to cmyers/moment that referenced this issue Nov 18, 2017
cmyers added a commit to cmyers/moment that referenced this issue Nov 18, 2017
cmyers added a commit to cmyers/moment that referenced this issue Nov 18, 2017
cmyers added a commit to cmyers/moment that referenced this issue Nov 18, 2017
cmyers added a commit to cmyers/moment that referenced this issue Nov 18, 2017
cmyers added a commit to cmyers/moment that referenced this issue Nov 21, 2017
marwahaha pushed a commit that referenced this issue Mar 2, 2018
…o global if missing (#4310)

* Fix #3883 defineLocale lazy load parentLocale

* Fix #3883 fallback to globalLocale if parentLocale can't be loaded,
added test

* Fix #3883 lazy load parentLocale test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment