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

feature: export the correct js file in npm #4391

Closed
wants to merge 2 commits into from
Closed

feature: export the correct js file in npm #4391

wants to merge 2 commits into from

Conversation

BernhardRode
Copy link

@BernhardRode BernhardRode commented Mar 9, 2017

When installing mapbox-gl via npm and importing the library like:

import * as mapboxgl from 'mapbox-gl';

The npm resolver is looking for tha main attribute in package.json to further resolve the correct JavaScript file for import.

fixes #3551

@lucaswoj
Copy link
Contributor

Our docs suggest using import mapboxgl from 'mapbox-gl/dist/mapbox-gl'; to import this module into Webpack.

That said, I’m leaning towards 👍 on this. It’d make require(‘mapbox-gl’) work in more cases. The only downside I can think of is if someone wants to npm link and see their changes instantly. That ought to be a pretty rare use case for GL JS since the monorepo.

Any thoughts @jfirebaugh?

@BernhardRode You'll need to fix the CI build & ensure npm publish still works.

@jfirebaugh
Copy link
Contributor

@mourner suggested this in #3551 and I'm in favor. Let's do it.

Copy link
Member

@mourner mourner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Let's fix the CI build, then merge.

@lucaswoj
Copy link
Contributor

CI fix:

diff --git a/package.json b/package.json
index c5b97fc..88338ee 100644
--- a/package.json
+++ b/package.json
@@ -113,7 +113,7 @@
     "build": "npm run build-docs # invoked by publisher when publishing docs on the mb-pages branch",
     "start-docs": "npm run build-min && npm run build-docs && jekyll serve --watch",
     "lint": "eslint --ignore-path .gitignore src test bench docs/_posts/examples/*.html debug/*.html",
-    "lint-docs": "documentation lint",
+    "lint-docs": "documentation lint src/index.js",
     "open-changed-examples": "git diff --name-only mb-pages HEAD -- docs/_posts/examples/*.html | awk '{print \"http://127.0.0.1:4000/mapbox-gl-js/example/\" substr($0,33,length($0)-37)}' | xargs open",
     "test": "run-s lint test-unit test-flow",
     "test-suite": "run-s test-render test-query",

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.

Make the distribution build the main NPM entry point
4 participants