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

Remove support for building mapbox-gl with webpack and other non-browserify bundlers #3235

Merged
merged 3 commits into from
Sep 21, 2016

Conversation

mourner
Copy link
Member

@mourner mourner commented Sep 21, 2016

Closes #3230 #3229 #2458 and will prevent a ton of new issues like this in future.

This makes all the fragile webpack hacks unnecessary, gives us the possibility to confidently depend on Browserify (e.g. for things like #3034), and saves us a lot of time on future tricky bugs and support requests. There's also seem to be no benefit to building Mapbox GL with another bundler. We'll also be free to switch to any other bundle system in future when we see fit without legacy support concerns.

The only trouble this brings is that Webpack will throw a warning, which does not make sense in our particular case:

WARNING in ./dist/mapbox-gl.js
Critical dependencies:
1:481-488 This seems to be a pre-built javascript file. Though this is possible, it's not recommended. Try to require the original source to get better results.

cc @jfirebaugh @lucaswoj @tmcw

@jfirebaugh
Copy link
Contributor

👍 Given webpack's architectural choices, this seems like the only sane choice to me.

Should we remove webpack.config.example.js too?

@mourner
Copy link
Member Author

mourner commented Sep 21, 2016

Ah, forgot to commit removal of the webpack config. Done.

Copy link
Contributor

@lucaswoj lucaswoj left a comment

Choose a reason for hiding this comment

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

🙌

@holman
Copy link

holman commented Oct 3, 2016

This fixed a lot of the problems I've been running into recently; thanks a bunch! 💟

As a sidenote for others running into this, if you're not down with the pre-build javascript file warnings you can suppress the errors for mapbox-gl's distribution build in your webpack config:

module: {
    noParse: /node_modules\/mapbox-gl\/dist\/mapbox-gl.js/,
    loaders: [{
    ...
    }]
}

captainbarbosa pushed a commit that referenced this pull request Oct 3, 2016
…serify bundlers (#3235)

* remove support for building mapbox-gl with webpack

* clarify usage of mapbox-gl with other module bundlers

* remove webpack config example
@yocontra
Copy link

If you can't switch to dist (a module you use uses mapbox-gl, not you) you can still use the old way! Update webworkify-webpack once borisirota/webworkify-webpack#21 is published and your existing stuff should work fine.

Alternatively, you can remove all of the old hacks and set up an alias which should work in most cases:

resolve: {
  alias: {
    'mapbox-gl/js/mapbox-gl.js': 'mapbox-gl/dist/mapbox-gl.js'
  }
}

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.

None yet

5 participants