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

Document incompatibility with Webpack #7196

Closed
stevage opened this issue Jul 6, 2020 · 7 comments
Closed

Document incompatibility with Webpack #7196

stevage opened this issue Jul 6, 2020 · 7 comments

Comments

@stevage
Copy link

stevage commented Jul 6, 2020

I just ran into the issue with marker images not displaying, due to some incompatibility with Webpack (#6585 and #4968).

I think it's fair to say that most modern web development uses build systems, and Webpack is the most popular. And the expectation that any library distributed on NPM works with Webpack unless otherwise stated is reasonable. To have Leaflet break in this way in such a bog-standard use case is very surprising. (Me, I'm using VueJS.)

I understand that the maintainers do not want to spend time fixing this incompatibility, or continuing to test for such issues.

However, could I suggest adding a clear statement either in the docs or even more prominently, that Leaflet is intended to be used directly without a build system, and that plugins may be required to operate with libraries such as Webpack.

@IvanSanchez
Copy link
Member

I understand that the maintainers do not want to spend time fixing this incompatibility, or continuing to test for such issues.

Do you want to spend time fixing this instead?

@stevage
Copy link
Author

stevage commented Jul 6, 2020

Not sure if that was a rhetorical question or not. My understanding from #4968 was that this was WONTFIX so I'm not sure my contribution would even be wanted.

I would be willing to document the incompatibility in a more visible way, if you would be receptive to such a PR.

@IvanSanchez
Copy link
Member

Not sure if that was a rhetorical question or not.

No it's not. Do give a shot at the code at

_detectIconPath: function () {

and try to come up with a way to detect the path of an asset that might or might not be copied around when bundling, while not increasing the size of the Leaflet releases, and that works the same with webpack, rollup, parcel, and browserify.

I would be willing to document the incompatibility in a more visible way, if you would be receptive to such a PR.

I'd just make a note in

// @option imagePath: String

then.

@stevage
Copy link
Author

stevage commented Jul 6, 2020

and try to come up with a way to detect the path of an asset that might or might not be copied around when bundling, while not increasing the size of the Leaflet releases, and that works the same with webpack, rollup, parcel, and browserify.

Maybe this didn't come across, but I really do believe that the maintainers have thoroughly examined this issue and there isn't an easy solution. All I'm suggesting is making this pitfall, and its straightforward workaround, more evident to future users of the library. I don't know if you're intending this, but I'm reading your response as sarcastic?

I'd just make a note in "Leaflet/src/layer/marker/Icon.Default.js" then

By "more visible" I had in mind, some or all of:

Is it possible for Leaflet to detect that it's being used inside Webpack? It could then at least output a warning, perhaps?

@mourner
Copy link
Member

mourner commented Jul 6, 2020

I really do believe that the maintainers have thoroughly examined this issue and there isn't an easy solution

Maybe there wasn't a few years ago, but if someone takes time to reexamine this thoroughly again with a fresh perspective (and the latest Webpack which evolved quite a bit), perhaps there could be an easy solution? I'm not sure because I don't use Webpack personally, but if there's a PR that makes things easier (either through docs or with some detection magic in the core), I'd be happy to merge it!

@IvanSanchez
Copy link
Member

Is it possible for Leaflet to detect that it's being used inside Webpack? It could then at least output a warning, perhaps?

A few minutes of research led me to https://stackoverflow.com/a/35785708 , which suggests that a hack involving typeof __webpack_require__ === 'function' would be possible. But that would cover only webpack, and not parcel/rollup/browserify.

@johnd0e
Copy link
Collaborator

johnd0e commented Jul 7, 2020

#7092?

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

No branches or pull requests

4 participants