globalizejs / globalize

A JavaScript library for internationalization and localization that leverages the official Unicode CLDR JSON data
https://globalizejs.com
MIT License
4.8k stars 603 forks source link

package.json: peerDependency no longer works #548

Open Mottie opened 9 years ago

Mottie commented 9 years ago

While performing an npm install the following message appears:

$ npm install npm WARN peerDependencies The peer dependency cldr-data included from globalize will no npm WARN peerDependencies longer be automatically installed to fulfill the peerDependency npm WARN peerDependencies in npm 3+. Your application will need to depend on it explicitly.

cldr-data@28.0.2 install D:\Repos\globalize\node_modules\cldr-data node install.js

Warning: Skipping to download CLDR data, because cldr-data is a peer dependency. If you want it to be downloaded, make sure it's listed under dependencies or devDependencies of the package.jsonof your application.

rxaviers commented 8 years ago

Hi, sorry for the delay. Please, could you provide context of the problem? I assume that when installing Globalize solely using npm intall, you npm install globalize cldr-data and when using it as a dependency of a project, you define both globalize and cldr-data as dependencies. In both cases, there's no npm complain.

Mottie commented 8 years ago

I see those messages if I use GitHub Desktop (button from code page 2016-01-19 15_11_56-jquery_globalize - javascript) to download the repository, or download a zip and save it to my drive.

Then, I use npm install to get npm to automatically grab all the needed dependencies, it won't include CLDR-data because it is a peer dependency. So, it requires an extra step of typing npm install cldr-data.

Trust me, it was a pain figuring all that out when you first start learning now to use npm.

rxaviers commented 8 years ago

Initially, I assumed you were talking about it from a user perspective, but I understand now that you are doing so from a contributor perspective developing Globalize. Now, I understand and confirm your issue. These warnings show up on npm@2 and npm@3 and it happens because cldr-data is a peer dependency (so, the declaration is correct), but we don't use it as devDependencies (which npm assumes we would).

I am open to suggestions on how to improve this for now. I think we'll only get rid of this problem when we actually start using the npm packages (instead of the bower ones.), which might happen when we update tests #450.

dvlsg commented 8 years ago

This also caused me a bit of a headache. Side note -- for me, even when executing npm install --save cldr-data@latest after a regular npm install, it didn't download the cldr data automatically. Same error as above.

However, when I removed globalize from package.json, I was able to save cldr-data and have it download the information, then re-added globalize to my package. Works as a workaround for me, in the meantime. I'm pretty sure I'm going to have issues as soon as I push to production, though, since I won't be able to hack apart my package.json to force an order onto npm install.

Looks like we could also list it as a devDependency if I'm reading this issue correctly?

rxaviers commented 8 years ago

Looks like we could also list it as a devDependency if I'm reading this issue correctly?

Globalize (as dev dependency) is currently using bower for cldr-data. If we added it in the package.json too, we would end up with two copies of cldr-data, which is significantly large.

dvlsg commented 8 years ago

Makes sense.

I have some additional information for reproducing the issue (if necessary). Turns out I stumbled across the issue originally by using npm install --save globalize followed by npm install --save cldr-data in that order.

If I flip the order to install cldr-data then globalize, or use npm install for a package.json containing both cldr-data and globalize at a clean location, the cldr-data is downloaded without issue.

I can't help but wonder if we're accidentally dodging a bullet simply because the c in cldr-data comes before the g in globalize. Here's one other way to reproduce, which leads me to think the download works based on the order in package.json. I executed these commands from a folder with an empty package.json:

npm install --save globalize cldr-data // does not download the cldr-data npm install --save cldr-data globalize // does download the cldr-data

rxaviers commented 8 years ago

@dvlsg it's important we distinguish the cases..

Above, I'm talking about the perspective of a contributor developing Globalize. It's an internal problem and doesn't affect (or shouldn't affect) users developing their apps using Globalize.

In your above example (chdir to a new empty directory / empty package.json) and using npm install --save globalize cldr-data, the perspective is from a user and this should work just fine. I confirm it works when package.json isn't present, but it behaves exactly like you mention with a new package.json for the following reason: cldr-data is large and in order to avoid unnecessary installations --- note npm@1 or npm@2 didn't install things flat as in npm@3, so in a normal installation one would end up with several cldr-data's --- I got cldr-data@73c8bce in place. There might be better solutions. Please, file an issue in cldr-data-npm with ideas on how to fix this or any other completely alternative ideas, e.g., cldr-data-npm/#28.

Thanks

dvlsg commented 8 years ago

Good point -- I'll move my part of the discussion to cldr-data. Sorry for misunderstanding and attaching my discussion to the wrong spot.

rxaviers commented 8 years ago

Thank you