search

cristiandley / react-gl-maps

Based on "tomchentw/react-google-maps" but with cool features added
MIT License
6 stars 5 forks source link

lib/creators/GoogleMapHolder.js was not rebuilt in last published patch #5

Open jwdotjs opened 8 years ago

jwdotjs commented 8 years ago

https://github.com/cristiandley/react-gl-maps/blob/master/lib/creators/GoogleMapHolder.js#L59

The last npm published patch didn't have the lib folder rebuilt from src for lib/creators/GoogleMapHolder.js so the styles key isn't there; I didn't realize /lib was part of the repo so I mistakenly thought it would be built with a prepublish command vs something I should have committed with my PR. My fault for not paying attention!

I can submit a PR with that built file unless you want to run build and republish

rewop commented 8 years ago

I believe a prebublish operation here is most appropriate. Also I would ignore the lib folder from the repo story at all.

There is no need to have the built code committed, as it can be reproduced, and forgetting to build before publishing is error prone and enhances the risk of publishing an old version in the library folder.

Regarding this issue to solve quickly we could republish with the newly built code.

cristiandley commented 8 years ago

@jwdotjs Im not going to be in a computer until next five hours, so if you make a PR with the build /lib i will be glad to merge it from my phone.

Im aware of the issue about having to build before pull. Can you expand the idea about ignoring it @rewop i like what you said... but im not an expert about npm packages in the other hand im open to implement and learn new things :)

rewop commented 8 years ago

@cristiandley the folder lib is a folder that can be recreated by transpiling the folder src with the command babel src --out-dir lib. This is exactly what the build script in package.json does.

This means that the lib folder needs to be only present in the machine in which the module is installed and used.

Now to obtain this there are two methods with npm:

  1. using postinstall script.
  2. using prepublish script.

If we use postinstall script, it means that the lib folder will be created when npm install is run in the machine the module is installed is being used in. In particular postinstall runs after all the dependencies in package.json are installed. This means we need babel in the dependencies, and we need to install babel also in a production environment, despite it is never used in the code at run time.

If we use prebublish script, the lib folder is built just before the module is published on npm. This means we don't need babel when the module is installed in the machine it is being used in. Therefore we can keep the dependency in the devDependencies without the need to install babel in production, and the module will be distributed directly with the lib folder.

Check here the documentation from npm.

In my opinion the second approach is cleaner. We could still run npm run build manually in development if needed.