add webpack demo app to show how to tree shake out only the methods in use

[jgravois]

i had a chat with @adamdbradley today 🎩 and he clued me in that consumers will have more luck tree-shaking our ES modules if TypeScript targets something newer than es5 during compilation.

i don't see a downside to this change because i'd expect those consumers to understand that they have to bring their own transpiler to the party anyway.

$ npx webpack
Hash: 2510a60b03d53bb95c2d
Version: webpack 4.29.6
Time: 1324ms
Built at: 03/15/2019 4:11:26 PM
    Asset      Size  Chunks             Chunk Names
bundle.js  1.77 KiB       0  [emitted]  main
Entrypoint main = bundle.js

instead of

$ npx webpack
Hash: df51c4225df7f519cb88
Version: webpack 4.29.6
Time: 1183ms
Built at: 03/15/2019 4:20:00 PM
    Asset      Size  Chunks             Chunk Names
bundle.js  6.53 KiB       0  [emitted]  main
Entrypoint main = bundle.js

AFFECTS PACKAGES:
@esri/arcgis-rest-webpack-demo

ISSUES CLOSED: #424

[jgravois]

its not necessary to call out the location of the tsconfig.json file explicitly.

the compiler searches for the tsconfig.json file starting in the current directory and continuing up the parent directory chain. - (reference)

[codecov]

Codecov Report

Merging #490 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master   #490   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          95     95           
  Lines        1214   1214           
  Branches      225    225           
=====================================
  Hits         1214   1214

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01a8a81...7fe9154. Read the comment docs.

[patrickarlt]

@jgravois I would like to review this but give me a few days.

[tomwayson]

Ditto. I'd like to try this out in an app I have that uses the ExB build.

I also wonder if we can do this in Hub.js first, since the stakes aren't as high there.

[patrickarlt]

@jgravois it looks like the --target es2017 flag does what I thought it did. The target param tells typescript output only code compliant with the specified target. This means that all ES2015 features like class are not compiled since they are assumed to be available under ES2017. This is why we see such a good reduction in size in this PR, TypeScript simply doesn't have to compile as much code. This also helps tree shaking algorithms figure out what code they can successfully drop since those algorithms don't have to parse through compiled code.

BUT this also comes at a cost which I think is too high. This will break in all browsers that don't support ES 2017. Which is pretty older versions of Chrome, Firefox, Safari, Edge and most importantly IE 11. This would be broken in these browsers until developers tell Babel (or maybe TypeScript) to compile code that is loaded from arcgis-rest-* down to ES5, since the default behavior for both Babel and TS is to not compile any code that comes from node_modules.

Since arcgis-rest-js is used in a TON of applications (ArcGIS for Developers included) I don't think we can make this change without at least a major version, but then people would still continue to use 1.x for IE 11 support.

Finally, our modules are REALLY small compared to what most developers will be using in a larger application and this change will only affect developers who are bundling with a really modern bundle like WebPack or Rollup and then most of the savings from this will shrink once that 4.76KiB that saved gets turned into ~1KiB or less after GZip

[jgravois]

thanks @patrickarlt.

i did some more snooping this morning and it appears that Stencil.js compiles the TypeScript for web components into both es5 and es2017 modules.

[patrickarlt]

@jgravois I suppose we could create 2 builds and do something like what https://philipwalton.com/articles/deploying-es2015-code-in-production-today/ proposes allowing users to use something like this:

<!-- Browsers with ES module support load this file. -->
<script type="module" src="main.mjs"></script>

<!-- Older browsers load this file (and module-supporting -->
<!-- browsers know *not* to load this file). -->
<script nomodule src="main.es5.js"></script>

This means we would essentially end up with the following builds:

Build	Module Format	ES Version	Notes
Node JS	Common JS	ES 5	Could possibly upgrade to 2015 and only support Node >= 6 or 2017 and only support Node >= 8
UMD	UMD	ES 5	For CDN, older browsers, and AMD/Browserify based build systems
ESM Legacy	ESM (EMCAScript Modules)	ES 5	This is the equivalent of our current ESM build.
ESM Modern	ESM(EMCAScript Modules)	ES2015 or ES2017	This is the proposed NEW build that users would have to opt into. They either have to configure Babel/TypeScript to compile for older browsers like babel/babel-loader#171 or use techniques like https://philipwalton.com/articles/deploying-es2015-code-in-production-today/ to handle both older + modern browsers. This could also be deployed to a CDN and used via script tags.

The main issue I could see here would be having to make 2 builds from the same code so that:

import {request} from "@esri/arcgis-rest-request";

in the ES5 build could seamlessly become:

import {request} from "@esri/arcgis-rest-request/modern";

In the ES2015/ES2017 build without having to make a bunch of code changes.

Overall though this still doesn't seem worth it to be for the size savings.

[jgravois]

thanks @patrickarlt.

this still doesn't seem worth it to be for the size savings.

for some of the packages here, you're probably right, but in hub.js land, our dependencies are really starting to pile up.

https://github.com/Esri/hub.js/blob/8c902e46e87cb11fc3a94eaf0bdce31fa18b4a88/packages/initiatives/package.json#L11-L24

[patrickarlt]

@jgravois I guess for the hub.js case it MIGHT make a difference. But I still think there isn't really a whole lot to tree shake out.

You can really cant shake anything out of @esri/arcgis-rest-request if you import request since that 1 function is basically the whole module. You can shake ApplicationSessionhttps://github.com/Esri/arcgis-rest-js/blob/master/packages/arcgis-rest-auth/src/ApplicationSession.ts out of @esri/arcgis-rest-auth but its only a ~120 lines and most of that is comments.

You would probably get more luck out of @esri/arcgis-rest-groups of which hub.js uses 5/11 of the main exported methods. but out build reports that @esri/arcgis-rest-groups is only 772 BYTES gzipped so if you can shake out half the module you have saved 386 bytes.

@esri/arcgis-rest-items and @esri/arcgis-rest-sharing have the biggest gains though. The Hub annotations module only uses searchItems protectItem and updateItem but the entirety of the items module is only 1.17kb when built so even a 90% savings only saves you ~1kb.

I just think the initial sizes are too small to really make meaningful gains. Is shaving 1kb or even 2kb off of hub.js worth the high cost in terms of changing our existing build process? I can't help but feel like this is a REALLY premature optimization for optimizations sake. The entirety of request, auth, items, groups and sharing is 7.68kb.

• request: 2.45 KB
• auth: 3.14 KB
• groups: 772 B
• items: 1.16 KB
• sharing: 1.16 KB
• Total: 7.68kb

With request and auth unable to be tree-shaken since most of their code is in UserSession and request there just isn't a lot of savings to be had and given the size of most frameworks shaving 1-2kb off of a build just really doesn't seem worth it. Most users could probably save far more by refactoring their CSS, compressing images or sub-setting their web fonts or cleaning up their other JS dependencies.

Don't get me wrong I love saving on size where we can I just don't think we should have 1-2Kb off some peoples builds is worth dropping a lot of compatibility. If we still want to do this we can but I think we need to do a clean break at v2 to do it and even then we will still need to maintain v1 for people who need to extra compat with older browsers.

[jgravois]

i appreciate you humoring me @patrickarlt. imagine this scenario:

in a Stencil.js web component, i want to:

import { followInitiative, unfollowInitiative } from "@esri/hub-initiatives";

the dependency graph for these methods includes:

• request()
• getUserUrl() - from @esri/arcgis-rest-auth
• a couple utility methods from hub-initiatives

with es5 modules, the following ends up being included in my bundle.

• request
• auth
• hub-initiatives - 4.3kb (of which i'm only using a small fraction)
• items - a 1.3kb dependency of hub-initiatives that i'm not touching
• groups - a 1kb dependency of hub-initiatives that i'm not touching
• sharing - a 1.3kb dependency of hub-initiatives that i'm not touching
• hub-domains a 300b dependency of hub-initiatives that i'm not touching
• hub-sites a 600b dependency of hub-initiatives that i'm not touching
• hub-common a 800b dependency of hub-initiatives that i'm not touching
• ES2017 polyfills that evergreen browsers don't need

I can't help but feel like this is a REALLY premature optimization for optimizations sake.

it may be, but my inclination as a developer, and i've seen customers doing the same thing, is to ignore that the function already exists in a higher order package and just rewrite it myself in my application using only request.

that works of course, but when users do this they miss out on a lot of the good work we've done.

as you said, the file sizes aren't that big. but listing all the modules below in my package.json when i only want to import two methods just feels like i'm including a lot more than i need. especially when Stencil is configured by default to create different bundles to serve up to ES2017 browsers and a bigger package to deliver to IE11 automatically.

---

The main issue I could see here would be having to make 2 builds from the same code so that:

import {request} from "@esri/arcgis-rest-request";

in the ES5 build could seamlessly become:

import {request} from "@esri/arcgis-rest-request/modern";

In the ES2015/ES2017 build without having to make a bunch of code changes.

to be honest, i had no idea something like this would be necessary. i just (optimistically) hoped we could add another target and surface it as yet another available compiled source in the package.json

"module:es2017": "dist/esm/es2017/index.js"

if that's the crux of the issue, i understand.

[patrickarlt]

tl;dr

• I agree we should do more to support treeshaking in bundlers
• I don't think we need a separate build to do it like @jgravois suggests
• We should add the sideEffects:false to our package.json to tell webpack and parcel that our modules can be aggressively treeshaken.
• Rollup perfectly treeshakes our code by default, which is awesome

@jgravois lets publish a new version of each module with sideEffects:false in all package.json files and see if that helps treeshaking in hub.js and stencil before trying to tackle a new ES2015/2017 build.

---

@jgravois ok I'll bite lets figure out how we can make this work. I forgot about the module fields in package.json. So on order to not break backward compat lets do this.

Update package.json scripts to add a new build:esm-modern that targets ES2017 so we now have 4 builds:

• Node
• UMD
• ESM compiled to ES5
• ESM compiled to ES2015

{
    "build:esm-modern": "tsc --module es2015 --target ES2015 --outDir ./dist/esm-2015 --declaration",
    "build:esm": "tsc --module es2015 --outDir ./dist/esm --declaration",
    "build:node": "tsc --module commonjs --outDir ./dist/node",
    "build:umd": "rollup -c ../../umd-base-profile.js && rollup -c ../../umd-production-profile.js"
}

Then we can declare our new build in package.json:

{
  "main": "dist/node/index.js",
  "unpkg": "dist/umd/auth.umd.js",
  "module": "dist/esm/index.js",
  "module:es2015": "dist/esm-2015/index.js",
  "js:next": "dist/esm/index.js",
  "types": "dist/esm/index.d.ts",
}

I'm pretty sure that Stencil should still tree-shake if we target ES2015. It is worth testing since far more people will be able to adopt a 2015 standard then a 2017 one. Also we aren't writing any
ES2017 code in the library to compile down anyway.

I also think the approach of adding a new module:es2015 field might be better then using the more standard module field. Webpack specifically mentions in https://webpack.js.org/guides/author-libraries/#final-steps that:

The module property should point to a script that utilizes ES2015 module syntax but no other syntax features that aren't yet supported by browsers or node.

My interpretation of this is that we should still ship ES5 in the module field and that NOT doing this might result in some parsers not being able to parse the module field at al. But doing this means we shouldn't break builds. If users want to opt into a better tree-shaking experience then they can add module:es2015 to their resolve.mainFields option in webpack. Rollup doesn't have a equivalent to resolve.mainFields until rollup/rollup-plugin-node-resolve#182 lands. If consumers do this they are then responsible for transforming the ES2015 code into whatever they need for compatibility.

However in doing all my research on this I found a few things:

• Webpack (and Parcel) support a sideEffects field in package.json. If we add sideEffects: false to our package.json files webpack will assume our file don't have any global scope and do more aggressive treeshaking by default in production mode without doing any of the above work.
• Rollup actually already treeshakes our ESM code by default VERY well. I tried a few examples in the REPL:
  • Remove ApplicationSession from auth module
  • Remove everything except searchItems

[patrickarlt]

@jgravois fix this and

I'll write something up in https://github.com/Esri/arcgis-rest-js/tree/master/notes or the wiki about treeshaking later and summarize all the research I did on this.

[patrickarlt]

@tomwayson TL:DR; adding sideEffects to all the modules and a proper webpack config allows webpack and parcel to treeshake our code REALLY efficiently. Rollup has some issues because of our use of tslib and removing tslib has its own set of trade offs. I'll do a full writeup of this in the notes or the wiki soon'ish to fully document everything.

[tomwayson]

I really appreciate all the thought you both have put into this and am looking forward to the writeup!