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

stop bundling node-pre-gyp #148

Merged
merged 1 commit into from
Jul 9, 2018
Merged

stop bundling node-pre-gyp #148

merged 1 commit into from
Jul 9, 2018

Conversation

springmeyer
Copy link
Contributor

@springmeyer springmeyer commented Jul 9, 2018

We no longer want to bundle node-pre-gyp. Bundling has the drawbacks of:

  • in a downstream module you don't get any security updates for node-pre-gyp dependencies, or node-pre-gyp yourself until you re-tag your module
  • recent versions of npm >=5 have started throwing errors when they encounter the bundled tree of node-pre-gyp. It seems like the tree expectations changed and recent npm borks frequently (especially when package-lock.js is involed). Instead of try to fix npm (or understand the problem more deeply), let's just recommend not bundling to get around this.

Note: prepublishOnly: npm ls is no longer valuable since its intention was simply to check that bundling looked correct before publishing.

So far this has been working in node-sqlite3 and a bunch of other modules I've released recently without any errors. So, its time to land it here as best practice.

/cc @mapbox/core-tech

refs mapbox/node-pre-gyp#403

We no longer want to bundle node-pre-gyp

refs mapbox/node-pre-gyp#403
@springmeyer springmeyer merged commit 616841d into master Jul 9, 2018
@springmeyer springmeyer deleted the unbundle branch July 9, 2018 01:32
springmeyer added a commit to mapbox/node-or-tools that referenced this pull request Jul 9, 2018
springmeyer added a commit to mapbox/mbtiles-geostats that referenced this pull request Jul 9, 2018
springmeyer added a commit to mapbox/carmen-cache that referenced this pull request Jul 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant