Esri / esri-leaflet

A lightweight set of tools for working with ArcGIS services in Leaflet. :rocket:
https://developers.arcgis.com/esri-leaflet/
Apache License 2.0
1.6k stars 798 forks source link

How do I not use the debug version? #1087

Closed timwis closed 6 years ago

timwis commented 6 years ago

Hiya folks! I'm using webpack-bundle-analyzer to inspect my bundle's dependencies and noticed that esri-leaflet-debug.js is one of the largest. I tried running webpack with NODE_ENV=production to see if there was a flag that made a difference, but it's still there. Any idea how to not use the debug version? (I assume the non-debug version is smaller).

screen shot 2018-04-09 at 6 38 45 pm

I'm importing it this way:

import { tiledMapLayer as EsriTileLayer } from 'esri-leaflet'
jgravois commented 6 years ago

greetings @timwis!

esri-leaflet-debug.js is the un-obfuscated/un-minified build of our lib this is true of leaflet-src.js as well.

the other reason our lib looks bloated in the chart is because we inline our sourcemaps for that file instead of writing them to *.js.map.

all that said, as i understand it webpack isn't using esri-leaflet-debug.js to import dependencies anyway, its using the raw ES6 source, so as long as you're obfuscating/minifying your own output and not inlining sourcemaps, regardless of the bundler (webpack, rollup, browserify), you are doing the requisite optimization.

next generation: tree shaking

ref: https://github.com/rollup/rollup/issues/541

i did some testing a few months ago to try and tree-shake both Leaflet and Esri Leaflet to only bundle source for imported classes (using the new experimental flags in both webback and rollup that supposedly allow you to ignore the possibility of unintended side effects) but I found no joy.

https://rollupjs.org/guide/en#experimental-options https://webpack.js.org/guides/tree-shaking/#mark-the-file-as-side-effect-free

timwis commented 6 years ago

Hey John! Thanks for the quick response. That makes sense, and it's cool to see this library shipping both versions. It looks like the issue was that by default, webpack gives precedence to the browser property, then module, then main. By changing the order of this in my webpack config, I was able to resolve the issue.

screen shot 2018-04-10 at 5 56 59 am

Up next is that pesky leaflet-src.js! Thanks for the suggestion on that -- I'll give it a shot. Cheers.

timwis commented 6 years ago

FYI it looks like leaflet ships an ESM version, but it doesn't actually come through when you npm install leaflet. I've posted an issue here if you're interested.

jgravois commented 6 years ago

even though at v1.3.1 leaflet didn't yet ship a flat ES6 bundle, their unbuilt code has been ES6 modules for awhile.

so just try adding "module": "src/Leaflet.js".

timwis commented 6 years ago

To a fork of leaflet? Or an alias of sorts in my webpack config?

jgravois commented 6 years ago

i meant setting a new value in /node_modules/leaflet/package.json in your local project for webpack to pick up on.

dadamssg commented 6 years ago

@timwis hey can you show exactly what you did in your webpack config to handle this? Something like this?

module.exports = {
  //...
  resolve: {
    mainFields: ['module', 'main', 'browser']
  }
}

This breaks my app. :/

timwis commented 6 years ago

Sure, I think if you clicked the commits linked above it should show you the exact changes I used. To be honest it was a few months ago and I don't remember. It may be worth cloning my Repository and using that bundle inspect tool to see if the leaflet dependency is actually smaller (use the v2 branch not master). Can't remember if I figured it out, but the reference commits suggest I may have.