kartena / Proj4Leaflet

Smooth Proj4js integration with Leaflet.
http://kartena.github.io/Proj4Leaflet/
BSD 2-Clause "Simplified" License
590 stars 173 forks source link

Work around Webpack import problem #147

Closed akx closed 7 years ago

akx commented 7 years ago

Note: The patch here is suboptimal; see https://github.com/akx/Proj4Leaflet/commit/a1c61f9e5e57c72ab59a89dc39b78518b538f995 for a better one!

When using Webpack, leaflet and proj4 may be exported as ES6 style modules that export the actual object as .default; so, if the modules have .default, use that.

The symptom for this issue is "TypeError: proj4.defs is not a function" (for ease of googling).

This is related to

perliedman commented 7 years ago

Hi, and thanks, but our position on this is that it's a Webpack issue, and not something we will hack around.

See discussion in #139 and #145.

akx commented 7 years ago

I understand your view there. Slippery slope and so on.

On the other hand this makes proj4leaflet unusable with (increasingly popular) systems such as create-react-app, which don't let you edit the Webpack configuration unless you "eject" the configuration and take full responsibility of it. 😥

perliedman commented 7 years ago

Maybe I don't fully understand the situation, but I guess it must also mean that Proj4 is unusable from create-react-app?

It seems that like a better solution to either fix Proj4, if their method of exporting isn't correct, or use a better Webpack configuration. Fixing it in Proj4Leaflet instead must be a hack?

pthorin commented 7 years ago

I think that this has to be either a problem for create-react-app or proj4, we are too far down the chain to fix this problem with webpack.

akx commented 7 years ago

proj4 works fine using ES6 module syntax in modules that are transpiled by Babel.

The ES6 code

import proj4 from 'proj4';
console.log(proj4);

outputs function proj4(fromProj, toProj, coord) { ... } in the browser console as expected. The code is transpiled into

'use strict';
var _proj = require('proj4');
var _proj2 = _interopRequireDefault(_proj);
function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }
console.log(_proj2.default);

by Babel -- that is, the object containing default is peeled into, if it's tagged as an ES module (by the convention Babel uses).

I'll dig a little deeper into what's happening – looks like Webpack is treating proj4leaflet as an AMD module, so this might not have fixed anything after all . . .

perliedman commented 7 years ago

Maybe a better solution would be to distribute Proj4Leaflet as a conventional CommonJS module, without UMD wrapper, and also include a "built" version with UMD wrapper for people who don't use a build tool and pull it in with a script tag?

akx commented 7 years ago

Okay, I think I got to the bottom of things here. The issue is in the interplay of Webpack (or any other ES6-compatible bundler really) and how Proj4 is packaged since the commit https://github.com/proj4js/proj4js/commit/7224e912ea0b7ca62bd0441946bed87c636169f4.

Act 1: The Module Field

Webpack honors the module field in package.json.

Act 2: Proj4

Proj4's package.json, referenced in the commit linked above, does contain a module field, so Webpack bundles it as one, i.e. with the default object (Proj4) exported as {default: Proj4}. This means that users would import it as import Proj4 from 'proj4';, and, well, see my earlier comment.

Act 3: Fixing Proj4leaflet

Because this issue stems from that "misunderstanding" in Webpack, distributing Proj4Leaflet as CJS would not really fix the issue (lest it is using ES6 import for the dependencies, which can then be transpiled by Babel with the interop require code).

I have a better, improved fix here (though I figured I wouldn't make a PR before I got green light from you guys first): https://github.com/akx/Proj4Leaflet/commit/a1c61f9e5e57c72ab59a89dc39b78518b538f995

Basically, it does the same thing interopRequireDefault would, and peels the default away if it detects Proj4 has been bundled as an ES6 module.

GeorgeKnap commented 7 years ago

@akx Hello, do you know if there is any progress about this issue? I've just run into it in my new app which uses

"leaflet": "1.2.0",
"proj4": "2.4.3", //2.3.x is not working either
"proj4leaflet": "1.0.1",
"webpack": "3.5.2"

and the proj4leaflet is calling proj4.defs(code, def); which does not exists because it is inside default property. All the related bug reports at webpack/proj4/proj4leaflet are closed but none of them seems to provide solution apart from blaming webpack. Feels to me like everyone who uses webpack/rollup must be stucked now like me. Your PR above looks promising but it's not released yet is it?

perliedman commented 7 years ago

For some reason I haven't even seen @akx's comment until now. Thanks for the detailed and thorough examination of the problem!

I see two options for us:

  1. Merge #145, which is more or less exactly the same patch as @akx link to above
  2. Turn Proj4Leaflet into a real ES6 module; we could the use the module field ourselves, which would be used by ES6 bundlers like Webpack, and we could point index to a bundled version with UMD wrapper; we could do the bundling with Rollup, exactly like Leaflet itself does from version 1.1.0 and forward

Option 2 seem like the good long term solution, but option 1 is way quicker, and personally I don't think I even have the time for that one. So, probably go with option 1 for now and make a release as soon as possible.

@pthorin any chance you have the time to look at this?

GeorgeKnap commented 7 years ago

For anxious people like myself, when can we expect the release?

perliedman commented 7 years ago

Published version 1.0.2 that contains this fix.

Thanks again @akx for the detailed write-up, really made me understand this problem; I think your fix is sufficient for now.

pthorin commented 7 years ago

Thanks @akx for this and @liedman for the release.

I will try to look into the option 2-solution after this week, when I'm back at work.

perliedman commented 7 years ago

@pthorin I thought about it, but I ended up thinking it's not worth it. I don't think anyone wins on us building and bundling stuff. The current method works for everyone and is easy to understand, although not elegant.

brgrz commented 6 years ago

For some reason, even after this fix usage in TS projects with import statements and Webpack build is broken. For instance:

import * as L from 'leaflet';
import * as MarkerCluster from 'leaflet.markercluster';
import * as proj4 from 'proj4';
import * as proj4leaflet from 'proj4leaflet';

Everything works fine except the proj4leaflet classes and functions. Apparently the Proj does not attach itself to the L and results in:

Cannot read property 'CRS' of undefined

at

mapOptions = {
    crs: new L.Proj.CRS(mapSettings.crs, mapSettings.projection,
        { resolutions: mapSettings.resolutions, origin: mapSettings.origin /*, zoomAnimation: true*/ })
}

If I switch to require statements, it works but I lose intellisense/typings support and also wouldn't like to change the way we load dependencies just in one component and just for the sake of proj4leaftlet.

GeorgeKnap commented 6 years ago

@brgrz

try this:

import * as proj4 from 'proj4leaflet';

mapOptions = {
    crs:  new proj4.CRS(mapSettings.crs, mapSettings.projection,
        { resolutions: mapSettings.resolutions, origin: mapSettings.origin /*, zoomAnimation: true*/ })
}
brgrz commented 6 years ago

@GeorgeKnap nope.. Property 'CRS' does not exist on type 'typeof ... ' (compile time error)

GeorgeKnap commented 6 years ago

@brgrz interesting. it works like this in my Angular app. Make sure you use proj4leaflet: 1.0.2

brgrz commented 6 years ago

For anyone else who might run into this, I made it work by just using

import * as L from 'leaflet';
import 'proj4leaflet';
zimmicz commented 6 years ago

Hi, I'm trying to use proj4leaflet with webpack 4 and am running into very similar issue.

Using these versions

"proj4": "~2.4.3",
"proj4leaflet": "^1.0.2",

I keep running into

proj4leaflet.js:52 Uncaught TypeError: proj4.defs is not a function
    at NewClass._projFromCodeDef (proj4leaflet.js:52)
    at NewClass.initialize (proj4leaflet.js:36)
    at new NewClass (leaflet-src.js:301)
    at NewClass.initialize (proj4leaflet.js:90)
    at new NewClass (leaflet-src.js:301)
    at Object.registerLeafletSrid (SridProvider.js:29)
    at app.js:132
    at Object.invoke (angular.js:5097)
    at angular.js:4894
    at forEach (angular.js:408)

I've tested thoroughly I think, and only thing that helps is changing https://github.com/kartena/Proj4Leaflet/blob/master/src/proj4leaflet.js#L18 to

if (proj4.default) {

Why is that? Is it possible to solve this differently?

machan201708 commented 5 years ago

For anyone else who might run into this, I made it work by just using

import * as L from 'leaflet';
import 'proj4leaflet';

hello,I have tried ,but it does not work when I use "npm run build"