No relative require in locales

[timaschew]

This PR rewrote all require('../moment') to require('moment')

The reason is to decouple the locales from the moment.js file.
This could cause problems in browser environment, because locale files are not always bundled with moment.js, so there should be no relative relation anymore.

In Node.js envirnoment this works fine, because node will lookup moment in the node_modules directory. Also browserify still works.

This makes it easier for tools which use require in browser environment to load moment. To get it working the namespace/path moment must be registered, instead of ../moment

[timaschew]

I found another solution instead of using npm link, use the new feature of npm 2: local dependencies: in my branch: https://github.com/timaschew/moment/commits/moment-as-module
see change in package:json

[ichernev]

I'm a bit confused now. Do we need this and why? What works better and what might (possibly) break.

[timaschew]

You should use it because moment and all locales files should obviously not bundled together in all cases. For instance if you want to load locale files via AMD or in some other dynamic way.
So this means, that the locales should be independent in the aspect of where moment is located.

So that involves that using ../moment in the require path should be avoided.

IMO the version with define a locale module in the package.json is cleaner than using grunt shell + npm link commands. The only condition is: you need npm >= 2.x
But since you need this only for test and development purposes, it should be ok.

[ichernev]

I haven't seen people complaining about the way we import moment inside language files. Also the way you propose to put it seems like the languages and core are in separate repositories. All node projects I've seen have relative dependencies between the files. If we put locale files in another repo, then it would make more sense, but it would also make it more complicated for users.

If this is causing a problem for component, why not maintain a separate component repository that we push on every release.

[timaschew]

Yes, a separate repos would make sense, but also a little bit overkill.

You talking about Node projects, but I'm talking about browser environment.
For server side it's no problem because file size doesn't matter in this dimension.
But in the browser/client world this is different.
This is not a problem for component only, this also a problem for browserify and every other browser build tool.

AMD has it's own syntax and define a namespace, this is the reason why their implementations like Require.js is not affected with this issue.

[timaschew]

I haven't seen people complaining about the way we import moment inside language files.

#2007

This and the problem of a dynamic require in the moment.js:

moment/moment.js

Line 777 in 981647d

require('./locale/' + name);

is the problem. There is only one environment where this works, it's Node.js.

[ichernev]

Well, the dynamic require is just a nice feature for node.js. Its not something that we want to support, it was just something that we could :)

How is size a problem for browser in this setting (browserify / component). They fetch only the files needed. Also for those environments -- yes, you have to specify the language files you want (or just take all locales) -- I don't see any problem with that. Is there a way around? Requiring a locale used to also set it, but then there are other weird cases we have to handle.

How is making the requires not relative fix any of those?

[ichernev]

The es6 locale is going to provide different builds for different environments, so then (hopefully) everybody will be happy, no matter the build / module system used.

You still didn't say which tools have problems loading moment now, and under what circumstances.

[timaschew]

You still didn't say which tools have problems loading moment now, and under what circumstances.

it's browserify (#2007) and component (#1966)

[ichernev]

@timaschew the require logic was severely changed in 2.10.0, can you verify if it works for you now (2.10.3 is latest).

I guess we still require relatively, but amd requires are absolute. I guess we can add a special browserify/component header that works well for it.

[dasa]

I've opened PR #3082 since it is sometimes a pain to define the main for the moment package. Is it still required that AMD use a package dependency?