Library bundling, filesize, etc

[mstenta]

The bundled farmOS-map.js file is pretty big. As of this writing, it clocks in at just over half a megabyte in size.

Most of this size comes from including various modules from the OpenLayers library. The approach we've taken with farmOS-map is to essentially create a pre-bundled, pre-configured, OpenLayers map that includes all the things that farmOS itself needs for its maps, along with some wrappers and helper functions for making it easier for farmOS modules to do common things (like add a WKT or GeoJSON layer). This approach allows us to create a standalone farmOS-map.js file that can be included in a non-JS application (like farmOS/Drupal).

The main drawback of this approach is that it's an "all or nothing" library. You can't, for example, say "I want farmOS-map.js with just the WKT and GeoJSON layer support". This sort of selective bundling would be really useful in the context of JS applications like Field Kit, which doesn't need everything that is included in farmOS-map.

It would be worth doing a quick test where we strip out everything that Field Kit doesn't need, build that, and see how much of a difference it actually makes. Maybe the "extras" are not as much as the "core" OpenLayers code itself, which would need to be included anyway. That would be a good first thing to test.

If that does make a significant difference, the next challenge is thinking about how to restructure this library so that it can more easily be imported into another JS application in pieces. Here is a quick list of the "pieces" that could potentially be made optional:

• Bundles "behaviors":
  • Google layers
  • Polygon/lineString measurement logic
  • Base layer localstorage memory
• Controls
  • Geolocate button (provi
  • Edit control (for drawing/modifying shapes)
  • LayerSwitcher
  • FullScreen
  • Rotate
  • ScaleLine
  • Geocoder
• Interactions
  • DragRotateAndZoom
  • PinchRotate
• Layer types
  • OpenStreetMap (the default base layer)
  • Group
  • Cluster
  • GeoJSON
  • WKT
  • WMS Tile
  • XYZ Tile
  • ArcGIS Tile (just added in #67 )
• Misc
  • Popups

[mstenta]

(FYI this has been something on our minds all along, but the discussion in #67 prompted creating this issue.)

[mstenta]

Also wanted to follow up on @jgaehring's comment in #67 : https://github.com/farmOS/farmOS-map/pull/67#issuecomment-608531558:

I think this brings into question the efficacy of using the global window.farmOS.map namespace in general. I know that solution seems to work best in the Drupal context, but perhaps we want to find a way to leave that decision up to the consumer, at the very least, or perhaps an alternative. Because if we don't require that global by default, it becomes trivial for the consumer to use their chosen bundler (eg, Webpack) to tree-shake unneeded dependencies. Hence, it wouldn't be our concern how many kb TileArcGISRest adds to the bundle. @mstenta, I'd love to hear again how the global is used within the Drupal context, and explore alternatives.

The window.farmOS.map global by itself is not really the main cause of filesize, although it would certainly make sense to re-evaluate making that optional in general. The main culprit is the instance object, which includes methods like addLayer() (and all of the various OL imports for different layer types), and addBehavior() (which includes behaviors like Google layers (BIG), edit controls (medium/big?), polygon/lineString measurement (small/medium), and base layer memory (small)).

So I think the first thing we would need to think through is how to make it possible to selectively import layer types and behaviors. Those seem like the pieces that would offer the biggest "wins" in terms of overall filesize.

[mstenta]

Ran a few quick tests of npm run build with various chunks of code commented out:

• No change (current file size): 518987 bytes
• Without behaviors (edit, google, measure, rememberLayer): 419656 bytes
• Without behaviors and without "extra" layer types (cluster, wms, arcgis-tile): 409979 bytes

So based on that (assuming I didn't miss anything), it looks like the behaviors are the big culprit, responsible for ~+100K. I wouldn't be surprised if Google is the biggest chunk of that.

But even so... the library is still 400K even with all that stuff disabled.

[mstenta]

If I also remove all "extra" default controls and interactions (LayerSwitcher, FullScreen, Rotate, ScaleLine, Geolocate, Geocoder, DragRotateAndZoom, and PinchRotate), the file size is: 349652 bytes

So those add ~50K.

[mstenta]

I pushed these three removal commits to a filesize-tests branch in my fork: https://github.com/mstenta/farmOS-map/tree/filesize-tests

[jgaehring]

The window.farmOS.map global by itself is not really the main cause of filesize, although it would certainly make sense to re-evaluate making that optional in general.

Just to clarify, I bring up the global object, not because that alone increases the filesize somehow, but because I believe it impairs Webpack's ability to automatically remove unused modules from the bundle when applications use farmOS-map. This is the "tree-shaking" concept: https://webpack.js.org/guides/tree-shaking/.

[mstenta]

Removing ONLY the google behavior takes the library from 518987 bytes to 474351 bytes (difference of ~45K).

This seems like a good first "low-hanging fruit" to start with. Field Kit does not use Google layers.

[mstenta]

I believe it impairs Webpack's ability to automatically remove unused modules from the bundle when applications use farmOS-map.

Oh interesting! So would Webpack be smart enough to NOT include Google code even though import OLGoogleMaps from 'olgm/src/olgm/OLGoogleMaps'; is in a file that IS included?

[jgaehring]

So would Webpack be smart enough to NOT include Google code even though import OLGoogleMaps from 'olgm/src/olgm/OLGoogleMaps'; is in a file that IS included?

Correct. I think the most reasonable thing for us to do is just to make sure the library doesn't break tree-shaking for the application code using it, so it's totally in the hands of the consumer which features they use or don't use, and which features get bundled or don't. It's less aggravation and fewer decisions for us, and more control in the hands of the consumer.

[mstenta]

The nice thing I've learned from these tests is that adding additional layer types (like ArcGIS Tile from #67 and potentially also WMTS from #63) don't really make a big difference. Removing cluster, arcgis-tile, and wms only shaved off 9677 bytes.

[mstenta]

Correct. I think the most reasonable thing for us to do is just to make sure the library doesn't break tree-shaking for the application code using it,

Ok, I'd love to do a rough test of this in Field Kit to see if it reduces the filesize of that in the same way!

But... I think Field Kit is using the global window.farmOS.map currently, right? So we can't really test it without diving into refactoring. Correct?

[jgaehring]

Yea, it would take some refactoring.

I wonder if it's worth trying to bundle the example index.html you have in this library to run tests?

[jgaehring]

I also wonder if it's worth having two separate main.js's: one where the map object get attached to the global window object, and one where instead the map object is the default export; the former could be used in Drupal, while the latter in Field Kit.

I think there are probably far better strategies, where we could optimize the bundle for Drupal as well, but this strikes me as the easiest solution for right now. If I understood better how Drupal uses that global, and had time to think about, I'm sure we could optimize for the Drupal context as well.

[mstenta]

I just ran one more test, where I only removed the pieces that Field Kit doesn't use/need (I think), including:

• Google layers
• Layer types: wms, arcgis-tile, cluster
• Controls: geocoder, fullscreen

That brings the library size from 518987 bytes to 423106 bytes, which is a difference of: 95881 bytes.

What this tells me (if assumptions are correct) is that if Webpack's "tree-shaking" can be made to work, that is roughly how much we would reduce the size of Field Kit.

I ran a quick npm run build:web of the latest develop branch of Field Kit, and the filesize of dist/static/js/1.0565bf093a3a8cbb6505.js is 1512846 bytes.

So back of the envelope math (rough, and admittedly there is more to this): 1512846 - 95881 = 1416965 bytes which is a 6% decrease.

That's not nothing. But as of right now I think we have more important things to work on. Saving 96K won't make that much of a difference, I don't think.

So I think my vote would be to shelve these ideas for now, and come back to it in the future. What do you think @jgaehring ?

I pushed these commits to a second branch: filesize-tests-fieldkit https://github.com/mstenta/farmOS-map/tree/filesize-tests-fieldkit

[mstenta]

I also wonder if it's worth having two separate main.js

That approach makes a lot of sense to me!

[jgaehring]

So I think my vote would be to shelve these ideas for now, and come back to it in the future.

Totally fine! I just wanted to make sure I registered my concern about the global, and that we didn't take any drastic measures or reject PR's on the basis of bundle size, if there were better alternatives. Let's circle back to this when things slow down. I wouldn't mind shaving off that 6%. I think the other big contributor is Bootstrap, but that's another issue.

[mstenta]

Agreed! And it's nice to have these numbers - they definitely put to rest my reservation about adding additional layer type options. That seems to be a very small contributor.

[mstenta]

The issue of the window.farmOS.map global and how it affects Webpack's ability to "tree shake" came up again in #72. Here is a quick overview of how that is being used currently and why:

In the context of farmOS (Drupal), maps need to be displayed in various contexts. And each context will want to have different features enabled on the map. For example:

• On the dashboard it shows a map of all areas, color coded by type, which you can click on to see details about that area.
• On asset listing pages (eg: /farm/assets/animals) it shows a map with the current location of animal assets represented as orange dashed-line polygons, on top of "all areas" in gray.
• On individual log records, a map is shown with the geometry stored on the log. And when you edit the log, a similar map is used but with the editor controls enabled.
• In Plan records (and if the farm_plan_map module is enabled), a map is displayed with the geometries of areas and assets that are linked to that plan.

Furthermore, and most importantly: farmOS modules can add additional features to maps and/or add their own maps to other places in the UI. For example:

• The farm_map_google module adds Google layers.
• The farm_map_mapknitter module adds Mapknitter layers.
• The farm_soil_nrcs module add the NRCS Soil Survey layer.
• The farm_plan_map module adds a map to Plan records.

We have no way of know which modules will be installed. And a site admin can enable/disable modules whenever they want, without any "build" process. This gets to the other important point...

Drupal does not run Webpack* - All it can do is add pre-made JS files to a given page.

(*It may be possible to figure out a way to run Webpack from Drupal, but there are a lot more considerations there, including added requirements for hosting/installing/updating/etc.)

So, in the farmOS/Drupal context... we basically have a situation where there is a map on a page, and any enabled modules can add things to that map. But the only way they can do that is by adding a JS file to the page.

(Oh and one other "furthermore": you can potentially have multiple maps on a page.)

Thus, the window.farmOS.map global was born. :-)

This global provides a few things:

• window.farmOS.map.instances is an array of map instances on the page. And "instance" is basically a JS object that farmOS-map defines, which has a map property (this is the actual OpenLayers map object), as well as some other properties and helper methods.
• window.farmOS.map.create() is a function for creating new map instances. This takes a target (DOM element ID), creates a new "instance" object, and adds it to the window.farmOS.map.instances array.
• window.farmOS.map.behaviors - This is an array of "behaviors", which are essentially just JS objects with an attach() method that is run when a map is created.

So, to outline how a map is built in farmOS:

1. The map is added to a page via the PHP farm_map_build() function. This adds some JS files to the page, including farmOS-map.js, which creates the window.farmOS.map global.
2. Other modules add their own JS files to the page, which add their "behaviors" to the window.farmOS.map.behaviors array.
3. When the page is finished loading in the browser (when jQuery .ready() fires), the farmOS map instance(s) are created by running window.farmOS.map.create().
4. When the create() method runs, it looks in window.farmOS.map.behaviors array, and runs the attach() method from each of them on each map instance.

Thus, modules can make modifications to maps by simply adding a JS file to the page.

That's the overview... hope it all makes sense. I'll save my "next steps" thoughts for future comments. I have some ideas... :-)

[mstenta]

Next steps:

Apart from the farmOS/Drupal (and similar) context, the other context farmOS-map could be used in is as a component of another JavaScript application. In that context, farmOS-map would be managed as a dependency via npm, and bundled into a final JS application file via Webpack.

In that context, the ability to add behaviors to a map via window.farmOS.map.behaviors is NOT necessary.

The ability to create map instances IS still necessary. Currently that is done via window.farmOS.map.create() - but we can find a way to separate that out as well.

Then... there's the matter of methods on the instance objects. This is where a lot of the other OL library modules are being imported. We may want to think about how we can make those imports optional too.

With all that in mind, I'm curious to understand which pieces in particular are currently preventing Webpack's "tree shaking". It feels to me like the global window.farmOS.map is less of an issue, because it is relatively thin. The instance objects and their methods might be the bigger culprit. (?)

[symbioquine]

Another important consideration here is the effect of extending farmOS-map on the final page size. As an example, I created a module which provides a snapping grid for use with farmOS-map. There doesn't seem to be a way to "link" such a module such that it can leverage the library code that farmOS-map already bundles. This means that the independently bundled code for farm_map_snapping_grid is more than 200KB.

[mstenta]

@symbioquine yea that's a great example of this tricky issue of bundling OpenLayers in farmOS-map, but wanting to re-use OpenLayers code in farmOS-map behaviors (or any other JS scripts) that are loaded dynamically on a page (not bundled with farmOS-map.js itself).

One idea there was to essentially expose some OpenLayers code on the window.farmOS.map object itself, so they could be reused by other scripts, post-build time. For example: window.farmOS.map.ol.source.Vector. Not sure I love that idea, but I can't think of a better alternative that ensures code isn't duplicated. We would also want to make sure that this is done in a way that it does not prevent the "tree-shaking" that @jgaehring describes in the cases where only pieces of farmOS-map are needed, and webpack can be used to bundle (eg: in Field Kit).

In farmOS itself, the considerations are different because we don't have the ability to bundle everything together. Modules can be added which provide their own pre-bundled JS files (like yours), and webpack is not part of the farmOS build/deployment process.

In the old days, you would just load ol.js on the page and that pulled in ALL of OpenLayers wholesale, which could then be re-used by any other script on the page (it created a window.ol global). I wonder if it would be worth considering making that an option again... eg: package a special version of farmOS-map.js that depends on ol.js being loaded on the page, and it uses that. The benefit of that is other scripts could use the full feature set of OpenLayers without duplicating that code in their own bundle. The downside, of course, is then we are loading the WHOLE OpenLayers library, which is probably bigger all around than all of these other options.

Those are all the "general" thoughts I have on the topic.... but getting back to your module specifically... I'm also curious if it would make sense to just merge that in as a behavior that farmOS-map itself provides. That would allow it to be bundled together. What do you think? Worth considering?

[symbioquine]

Those are all the "general" thoughts I have on the topic....

Those are great points. What about the idea of running the Webpack build on module installation? I think it was mentioned above, but I'm not sure I understood whether that concept was already thoroughly explored...

but getting back to your module specifically... I'm also curious if it would make sense to just merge that in as a behavior that farmOS-map itself provides. That would allow it to be bundled together. What do you think? Worth considering?

Absolutely! I only published it as a separate module to gather an initial round of feedback and get it into production for my use-case quicker.

I consider the current implementation a bit of a prototype. In particular I'd like feedback on the UI/UX choices that I made and whether the grid size normalization which I'm doing to convert between flat/spherical coordinates introduces too much error for folks in practice. I'll open an issue to track this potential integration in the farmOS-map issue queue.

[mstenta]

What about the idea of running the Webpack build on module installation? I think it was mentioned above, but I'm not sure I understood whether that concept was already thoroughly explored...

It hasn't been thoroughly explored. But it definitely introduces complexity for self-hosting farmOS. I'd love to think it through though, perhaps in a separate issue, to map out all the considerations.

I'd also be curious to see if the Drupal community at large has put any thought into this, because it is a general Drupal deployment question in my mind. It's certainly possible, it would just come down to the trade-offs involved. And one of my goals is always trying to keep the hosting process relatively simple so it doesn't become more of a barrier to entry for self-hosting.

[symbioquine]

What about the idea of running the Webpack build on module installation? I think it was mentioned above, but I'm not sure I understood whether that concept was already thoroughly explored...

It hasn't been thoroughly explored. But it definitely introduces complexity for self-hosting farmOS. I'd love to think it through though, perhaps in a separate issue, to map out all the considerations.

I'd also be curious to see if the Drupal community at large has put any thought into this, because it is a general Drupal deployment question in my mind. It's certainly possible, it would just come down to the trade-offs involved. And one of my goals is always trying to keep the hosting process relatively simple so it doesn't become more of a barrier to entry for self-hosting.

It looks like there's already some existing work on this front in the larger Drupal community targeting >= D8; https://www.drupal.org/project/webpack Would it make sense to keep a tracking issue for that potential integration in the main farmOS issue queue?

[mstenta]

Would it make sense to keep a tracking issue for that potential integration in the main farmOS issue queue?

Makes sense to me! Thanks for digging in @symbioquine !

[symbioquine]

Thinking about this more...

Goals

1. Ensure farmOS-map continues to encapsulate the typical setup/defaults for OpenLayers maps associated with farmOS
2. Make farmOS-map easily configurable/extensible - ideally without restricting the feature-set of OpenLayers unnecessarily
3. Make it trivial for pages/applications using formOS-map to be efficient with browser/network resources

Constraints

1. farmOS-map currently may have up to three distinct deployment "modes"
   • As distributed with farmOS (1.x/2.x)
   • As bundled into another application e.g. FieldKit
   • As a single js file distributed with other applications or loaded via CDN
2. The combination of configuration/extensions with which farmOS-map may run is variable by use-case and unpredictable at build-time

[symbioquine]

It occurs to me that this shouldn't be a new problem, lots of JS applications likely incorporate large foundational libraries and need to be extensible without fully abstracting those libraries. We may not be able to find prior art specifically for OpenLayers, but we shouldn't have to start from scratch...

For instance googling "packaging angular in extensible app" yields some interesting leads;

• https://stackoverflow.com/questions/41438198/implementing-a-plugin-architecture-plugin-system-pluggable-framework-in-angu
• https://medium.com/angular-in-depth/building-extensible-dynamic-pluggable-enterprise-application-with-angular-aed8979faba5

[mstenta]

In some ways our "behavior" system is very flexible in that regard. It just looks for JS objects within a given global namespace. These can be added by "packaged" JS like farmOS-map itself, or by other JS files that are just added to the page at the same time (like in farmOS itself).

It seems that including the lower-level OpenLayers dependencies is the biggest factor here. Maybe we should add an option that builds farmOS-map WITHOUT the OpenLayers dependencies, and instead assume that ol.js is added to the page already.

Then it's up to the app to decide what works best for them. farmOS itself could include the whole OpenLayers library alongside the minimal farmOS-map.js file, and then any other behaviors would have access to everything in the global OpenLayers namespace.

Or... something like Field Kit could use more standardized methods of including only the pieces they need, so the final built JS file is as small as possible.

[mstenta]

(Easier said than done, I'm sure... it would have to figure out how to deal with those differing namespaces everywhere in the library somehow...) :-)

[symbioquine]

(Easier said than done, I'm sure... it would have to figure out how to deal with those differing namespaces everywhere in the library somehow...) :-)

Agreed. I think it would have to do something like the repackaging described here: https://github.com/Turbo87/sidebar-v2/issues/143#issuecomment-617928867

[symbioquine]

It seems that including the lower-level OpenLayers dependencies is the biggest factor here. Maybe we should add an option that builds farmOS-map WITHOUT the OpenLayers dependencies, and instead assume that ol.js is added to the page already.

Then it's up to the app to decide what works best for them. farmOS itself could include the whole OpenLayers library alongside the minimal farmOS-map.js file, and then any other behaviors would have access to everything in the global OpenLayers namespace.

Definitely and option worth considering.

Pros

• Conceptually simple
• Doesn't impose specific build/development patterns on contrib modules - except those already present in terms of writing a "behavior"
• Guarantees all of OpenLayers is available for use in contrib modules

Cons

• Makes it possible to have version mismatches where wrong/incompatible versions of ol.js are used
• Innefficient - every (uncached) pageload incurs at minimum a 100KB penalty for that flexibility
  • farmOS-map.js@1.4.2 is 554.32KB or 129.89KB compressed
  • ol.js@6.5.0 is 972.47KB or 224.47KB compressed

[mstenta]

Good summary.

farmOS-map.js@1.4.2 is 554.32KB or 129.89KB compressed

Presumably this would be smaller without the OL stuff though?

But agreed that's the main tradeoff. In order to have ultimately flexibility, the whole OL library would be necessary.

[symbioquine]

---

OpenLayers already provides a super elegant extension model wherein custom controls, interactions, etc can be registered with the Map and even have complex dependencies between themselves.

First off: What value does the 'behaviors' abstraction provide on top of the extension model OpenLayers already provides?

Pros

• Provides a mechanism outside of specific OpenLayers map instances to accumulate "units of functionality" which should be applied to all farmOS-map instances when they get created
• Super simple - only exposes an attach method entry-point and a weight attribute to control loading order
• Allows behaviors to hook into any part of a farmOS-map instance either producing immediate changes to the functionality or hooking into events to surface at the right time

Cons

• Dependencies between behaviors are complex
  • Some combination of loading order and event listening has to be used to ensure access/interactions only occur when they are valid
  • Behaviors are entirely at the mercy of the page to supply any related/dependency behaviors

All that said, I think the current "behaviors" abstraction is a pretty good fit given the expectation that behaviors contain coarse-grained units of functionality which must roll their own interoperability mechanisms where necessary.

---

Stripping everything except the behavior system and the most basic map/view creation code, yields a farmOS-map.js bundle that is just under 2KB!

This could be really awesome in terms of being very flexible and allowing all the other functionality to be pulled into separate behaviors, however there is also a corresponding cost of maintaining the packages/modules for each and having a greater dependence on behavior loading order. For example to ensure support for a given layer type is loaded before a behavior that needs to add layers of that type.

[symbioquine]

One way to reduce that maintenance cost somewhat is to keep some of the behaviors within the farmOS-map source tree, but move to building them as separate bundles.

Based on that idea, I'm proposing the following package/bundle organization;

farmOS-map/
            farmOS-map.js - Contains only core behavior system + code to create the map/view
            10-farmOS-map-projection.js - Exposes data/feature projection constants: `farmOS.map.proj.(data|feature)`
            15-farmOS-map-util-forEachLayer.js - Exposes `farmOS.map.utils.forEachLayer`
            16-farmOS-map-util-measure.js - Exposes `farmOS.map.utils.(formatLength|formatArea|measureGeometry)`
            20-farmOS-map-default-styles.js - Loads basic default css and exposes default color OL `Style` object somehow
            25-farmOS-map-layer-switcher.js - Adds the layer switcher control
            30-farmOS-map-remember-layers.js - Remembers enabled base layers
            40-farmOS-map-fullscreen.js - Adds the fullscreen control
            41-farmOS-map-rotate.js - Adds the rotate control
            42-farmOS-map-scale-line.js - Adds the scale-line control
            43-farmOS-map-geolocate.js - Adds the geolocate control
            44-farmOS-map-geocoder.js - Adds the geocoder control
            50-farmOS-map-edit.js - Provides standard edit controls + drawing layer
            55-farmOS-map-measure.js - Adds measurements to drawing layers
            60-farmOS-map-default-navigation.js - Provides the navigation interactions (DragRotateAndZoom/PinchRotate)
            61-farmOS-map-snapping-grid.js - Provide snapping grid Control/interaction
            70-farmOS-map-osm.js - Provide Open Street Map base layers
            71-farmOS-map-google-maps.js - Provide Google Maps base layers

Note: I've named the modules to include the proposed weights. It would be a little redundant, because the weight would be defined within the exported behavior object, but it would also make the loading order more obvious and a build step could enforce that the number in the filename matches the one in the behavior object.

There's of course an argument for moving some of these things to their own modules - especially the Google maps one - but this strategy means it shouldn't be a size based argument - only a maintenance one.

[mstenta]

I think the current "behaviors" abstraction is a pretty good fit given the expectation that behaviors contain coarse-grained units of functionality which must roll their own interoperability mechanisms where necessary.

Yea this is exactly it. Aside from the standard attach() method, behaviors can also define their own methods and logic, and encapsulate all of that in a reusable way. So it is a level of abstraction above the OpenLayers extension model.

Stripping everything except the behavior system and the most basic map/view creation code, yields a farmOS-map.js bundle that is just under 2KB!

Phew! How about that! :-D

Based on that idea, I'm proposing the following package/bundle organization;

I like this way of thinking. The behaviors in the list above might be a bit more granular than we need, but that's open for discussion. Overall I think it's a great approach.

Note: I've named the modules to include the proposed weights.

Yea not sure I love the redundancy here - but that's just me. If weight needed to be changed it would mean moving the file and changing a line in it, which makes for messier diffs IMO (and you need to remember to do both). But that's a nit.

There's of course an argument for moving some of these things to their own modules - especially the Google maps one

Yes - I'd definitely like to consider splitting the Google stuff out to a separate repository. See related: #99

We are also considering doing the same with the farm_map_google module that is packaged in farmOS. We could consider just putting this behavior directly into that module.

but this strategy means it shouldn't be a size based argument - only a maintenance one.

:+1:

[symbioquine]

Stripping everything except the behavior system and the most basic map/view creation code, yields a farmOS-map.js bundle that is just under 2KB!

Phew! How about that! :-D

Based on that idea, I'm proposing the following package/bundle organization;

I like this way of thinking. The behaviors in the list above might be a bit more granular than we need, but that's open for discussion. Overall I think it's a great approach.

Yeah, I've been going back and forth on the level of granularity...

There's also a version I've been playing with that skips the util behaviors in favor of keeping those methods in the core. That approach clocks in at a little less than 20KB. (With ~10KB being from that popup library.)

Some of the finer granularity has additional benefits as well. For example, putting the projection and styles in their own behaviors would allow those to be trivially replaced - especially if we create corresponding farmOS modules for those behaviors so switching to a different projection/style is just a matter of disabling the default module and providing one's own...

We are also considering doing the same with the farm_map_google module that is packaged in farmOS. We could consider just putting this behavior directly into that module.

That sounds great!

Note: I've named the modules to include the proposed weights.

Yea not sure I love the redundancy here - but that's just me. If weight needed to be changed it would mean moving the file and changing a line in it, which makes for messier diffs IMO (and you need to remember to do both). But that's a nit.

Yeah, I debated this a fair bit. I think there's a lot to be said for having them sorted and having the weight be explicit in the behavior code - so it's not too magical. (As it might be if the weight came from the filename at build time.)

which makes for messier diffs

I think I can test how bad this would be... git/Github can be pretty clever sometimes at showing moved and slightly modified files.

and you need to remember to do both

That's why I was suggesting some sort of built-time validation to ensure they don't get out of sync :)

[mstenta]

Another thing to consider with weights is the ability for downstream code to override them.

And one more idea that pops to mind: if we have very granular behaviors, perhaps higher-level behaviors could be created that just combine a bunch of lower-level ones. For example: a behavior called "default behaviors" or something, which includes all the "defaults" we enable now: https://github.com/farmOS/farmOS-map/blob/1.x/src/instance/defaults.js

[symbioquine]

Another thing to consider with weights is the ability for downstream code to override them.

Would that be a good thing? Can you give an example where somebody might want to this?

And one more idea that pops to mind: if we have very granular behaviors, perhaps higher-level behaviors could be created that just combine a bunch of lower-level ones. For example: a behavior called "default behaviors" or something, which includes all the "defaults" we enable now: https://github.com/farmOS/farmOS-map/blob/1.x/src/instance/defaults.js

This starts to get into modeling dependencies - which I thought we were trying to avoid with the weight system. Of course we could pull in some sort of lightweight dependency system like modulejs for that...

[mstenta]

Would that be a good thing? Can you give an example where somebody might want to this? ... This starts to get into modeling dependencies - which I thought we were trying to avoid with the weight system. Of course we could pull in some sort of lightweight dependency system like modulejs for that...

Good points... hard to say... just thoughts that popped to mind.

[symbioquine]

I've published a few commits now to the WIP change set in https://github.com/farmOS/farmOS-map/pull/109 Would love some feedback to see whether this direction makes sense to other folks.

• @mstenta I diverged a bit from the strategy I described above, since I discovered some Webpack features that make this a way smaller change in a lot of ways...
• @jgaehring I haven't tested how this will work from other bundled apps, but I think it should...

[symbioquine]

Just a minor update here. I tested FieldKit against these WIP changes. It only required a few code changes to work against the new version;

diff --git a/package.json b/package.json
index 00ee846..e7d6022 100644
--- a/package.json
+++ b/package.json
@@ -29,8 +29,8 @@
     "cordova-plugin-network-information": "^2.0.2",
     "cordova-plugin-splashscreen": "^5.0.3",
     "cordova-plugin-whitelist": "^1.3.4",
-    "farmOS-map": "github:farmOS/farmOS-map#v1.3.0",
     "farmos": "^0.1.6",
+    "farmOS-map": "git://github.com/symbioquine/farmOS-map.git#2.x",
     "ramda": "^0.27.0",
     "vue": "^2.6.10",
     "vue-router": "^3.1.3",
diff --git a/src/components/FarmMap.vue b/src/components/FarmMap.vue
index 521e19f..0fe6f80 100644
--- a/src/components/FarmMap.vue
+++ b/src/components/FarmMap.vue
@@ -7,9 +7,11 @@
 </template>

 <script>
-import 'farmOS-map/src/main';
+import farmOSMap from 'farmOS-map/src/main';
 import { mapState } from 'vuex';

+import 'ol/ol.css';
+
 export default {
   name: 'FarmMap',
   data() {
@@ -63,7 +65,7 @@ export default {
     mapboxAPIKey: state => state.shell.mapboxAPIKey,
   }),
   mounted() {
-    this.map = window.farmOS.map.create(this.id, this.options);
+    this.map = farmOSMap.create(this.id, this.options);
     if (this.geojson) {
       this.layers.geojson = this.map.addLayer('geojson', this.geojson);
     }
@@ -72,6 +74,24 @@ export default {
     const layerWeights = this.wkt.map(e => (e.wkt
       && e.wkt !== 'GEOMETRYCOLLECTION EMPTY'
       ? e.weight : 99));
+
+    const refireWktEditEvents = () => {
+      if (this.drawing && this.map.edit) {
+        this.map.edit.wktOn('drawend', (wkt) => {
+          this.$emit('update-wkt', wkt);
+        });
+        this.map.edit.wktOn('modifyend', (wkt) => {
+          this.$emit('update-wkt', wkt);
+        });
+        this.map.edit.wktOn('translateend', (wkt) => {
+          this.$emit('update-wkt', wkt);
+        });
+        this.map.edit.wktOn('delete', (wkt) => {
+          this.$emit('update-wkt', wkt);
+        });
+      }
+    };
+
     this.wkt.forEach((wktElement) => {
       // Zoom to the layer if it has the lowest weight
       if (!this.drawing
@@ -88,7 +108,8 @@ export default {
           if (wktElement.weight === Math.min(...layerWeights) && wktElement.canEdit) {
             this.layers[wktElement.title] = this.map.addLayer('wkt', wktElement);
             this.map.zoomToLayer(this.layers[wktElement.title]);
-            this.map.addBehavior('edit', { layer: this.layers[wktElement.title] });
+            this.map.addBehavior('edit', { layer: this.layers[wktElement.title] })
+                .then(refireWktEditEvents);
             this.map.addBehavior('measure', { layer: this.layers[wktElement.title] });
           } else if (wktElement.weight === Math.min(...layerWeights)) {
             this.layers[wktElement.title] = this.map.addLayer('wkt', wktElement);
@@ -98,25 +119,13 @@ export default {
           }
           hasLayers = true;
         } else {
-          this.map.addBehavior('edit');
-          this.map.addBehavior('measure', { layer: this.map.edit.layer });
+          this.map.addBehavior('edit').then(() => {
+            this.map.addBehavior('measure', { layer: this.map.edit.layer });
+            refireWktEditEvents();
+          });
         }
       }
     });
-    if (this.drawing && this.map.edit) {
-      this.map.edit.wktOn('drawend', (wkt) => {
-        this.$emit('update-wkt', wkt);
-      });
-      this.map.edit.wktOn('modifyend', (wkt) => {
-        this.$emit('update-wkt', wkt);
-      });
-      this.map.edit.wktOn('translateend', (wkt) => {
-        this.$emit('update-wkt', wkt);
-      });
-      this.map.edit.wktOn('delete', (wkt) => {
-        this.$emit('update-wkt', wkt);
-      });
-    }
     if (!hasLayers) {
       this.map.zoomToVectors();
     }
@@ -132,7 +141,7 @@ export default {
     }
   },
   beforeDestroy() {
-    window.farmOS.map.destroy(this.id);
+    farmOSMap.destroy(this.id);
     this.map = null;
   },
   watch: {

Obviously, that refireWktEditEvents logic isn't necessarily right because if there were two edit layers it would subscribe to WKT events for each whereas the original logic only subscribed to the last one...

There was also some styling issue that I haven't fully figured out, but overall it seems super promising. The FieldKit build even automatically loaded the behaviors as separate vendor chunks - so the edit behavior only loaded the first time a user clicks through on the map to create a custom geometry.

[symbioquine]

Similarly, the changes to make farmOS 2.x work with the WIP PR changes are pretty light-weight;

diff --git a/modules/core/map/farm_map.libraries.yml b/modules/core/map/farm_map.libraries.yml
index 82f0dc59..05ff8047 100644
--- a/modules/core/map/farm_map.libraries.yml
+++ b/modules/core/map/farm_map.libraries.yml
@@ -5,8 +5,14 @@ farmOS-map:
     url: https://github.com/farmOS/farmOS-map/blob/master/LICENSE
     gpl-compatible: true
   js:
+    /libraries/farmOS-map/dist/ol.js:
+      minified: true
     /libraries/farmOS-map/dist/farmOS-map.js:
+      type: external
       minified: true
+  css:
+    theme:
+      /libraries/farmOS-map/dist/ol.css: { }
   dependencies:
     - core/drupalSettings

diff --git a/modules/core/map/js/farmOS.map.behaviors.geofield.js b/modules/core/map/js/farmOS.map.behaviors.geofield.js
index 335b91f9..108b83ba 100644
--- a/modules/core/map/js/farmOS.map.behaviors.geofield.js
+++ b/modules/core/map/js/farmOS.map.behaviors.geofield.js
@@ -1,9 +1,11 @@
 (function () {
   farmOS.map.behaviors.geofield = {
     attach: function (instance) {
-      instance.edit.wktOn('featurechange', function(wkt) {
-        console.log('here!');
-        document.querySelector('#' + instance.target).parentElement.querySelector('textarea').value = wkt;
+      instance.editAttached.then(() => {
+        instance.edit.wktOn('featurechange', function(wkt) {
+          console.log('here!');
+          document.querySelector('#' + instance.target).parentElement.querySelector('textarea').value = wkt;
+        });
       });
     },

diff --git a/modules/core/map/js/farmOS.map.behaviors.wkt.js b/modules/core/map/js/farmOS.map.behaviors.wkt.js
index be343c79..1060522c 100644
--- a/modules/core/map/js/farmOS.map.behaviors.wkt.js
+++ b/modules/core/map/js/farmOS.map.behaviors.wkt.js
@@ -17,29 +17,35 @@
         var layer = instance.addLayer(type, opts);
       }

+      var focusLayerPromise = Promise.resolve(layer);
+
       // If edit is true, enable drawing controls.
       if (drupalSettings.farm_map[instance.target].behaviors.wkt.edit) {
         if (layer !== undefined) {
-          instance.addBehavior('edit', { layer: layer });
+          instance.editAttached = instance.addBehavior('edit', { layer: layer });
         } else {
-          instance.addBehavior('edit');
-          var layer = instance.edit.layer;
+          instance.editAttached = instance.addBehavior('edit');
+          // Focus on the edit layer if no layer was provided
+          focusLayerPromise = instance.editAttached
+            .then(() => instance.edit.layer);
         }

         // Add the snappingGrid behavior.
         instance.addBehavior('snappingGrid');
       }

-      // Enable the line/polygon measure behavior.
-      instance.addBehavior('measure', { layer: layer });
+      focusLayerPromise.then(focusLayer => {
+        // Enable the line/polygon measure behavior.
+        instance.addBehavior('measure', { layer: focusLayer });

-      // If the layer has features, zoom to them.
-      // Otherwise, zoom to all vectors.
-      if (layer !== undefined) {
-        instance.zoomToLayer(layer);
-      } else {
-        instance.zoomToVectors();
-      }
+        // If the layer has features, zoom to them.
+        // Otherwise, zoom to all vectors.
+        if (focusLayer !== undefined) {
+          instance.zoomToLayer(focusLayer);
+        } else {
+          instance.zoomToVectors();
+        }
+      });
     },
     weight: 100,
   };

Notes:

• These changes assume that ol.js/ol.css are in the dist folder of farmOS-map, but we'd probably want to do that differently.
• farmOS.map.behaviors.geofield.js depends on the property instance.editAttached which is written by farmOS.map.behaviors.wkt.js. There's probably a better strategy for that...
• farmOS-map.js is set to type: external so that it is never combined with other JS since that breaks the chunk loading. This should usually work, but might break if farmOS is hosted in a subpath instead of at /.

[symbioquine]

• farmOS-map.js is set to type: external so that it is never combined with other JS since that breaks the chunk loading. This should usually work, but might break if farmOS is hosted in a subpath instead of at /.

Actually it looks like a better strategy is using preprocess: false which produces similar results, but shouldn't break if farmOS were hosted at a subpath.

diff --git a/modules/core/map/farm_map.libraries.yml b/modules/core/map/farm_map.libraries.yml
index 82f0dc59..8a9928ab 100644
--- a/modules/core/map/farm_map.libraries.yml
+++ b/modules/core/map/farm_map.libraries.yml
@@ -5,8 +5,16 @@ farmOS-map:
     url: https://github.com/farmOS/farmOS-map/blob/master/LICENSE
     gpl-compatible: true
   js:
+    /libraries/farmOS-map/dist/ol.js:
+      minified: true
     /libraries/farmOS-map/dist/farmOS-map.js:
+      # Skip aggregating farmOS-map.js with other JS since that
+      # breaks the lazy loading of behavior chunks.
+      preprocess: false
       minified: true
+  css:
+    theme:
+      /libraries/farmOS-map/dist/ol.css: { }
   dependencies:
     - core/drupalSettings

[mstenta]

I've reviewed all the proposed changes in #109 and they look really good @symbioquine ! I like where this is headed!

Perhaps it makes sense to organize/summarize the "next steps" (maybe in the PR description?) so we can be sure everything is accounted for... including relevant PRs for Field Kit and farmOS 2.x.

Just a couple of notes I made while reviewing:

• We should include some instructions for updating from 1.x to 2.x, for anyone who's using farmOS-map v1 in their projects currently. I know there are a few out there aside from farmOS and Field Kit.
• Minor nitpicks / coding standards:
  • Changelog items should end in periods.
  • There are some extra newlines in README.md.
  • Needs a newline above comment inside attachBehavior().
  • I haven't fully grokked the behavior sort logic - I wonder if we could add some comments, and maybe move it out to an include file? I like that main.js is so small/readable otherwise.

[mstenta]

Would it make sense to add an agenda item to discuss these changes and next steps on the 5/12 monthly call (this Wednesday)? I don't think we have anything else planned.

Or, if we want to find a separate meeting time that works too. Would love to have both @paul121 and @jgaehring join.

[symbioquine]

Awesome, thanks for looking over it all @mstenta!

Perhaps it makes sense to organize/summarize the "next steps" (maybe in the PR description?) so we can be sure everything is accounted for... including relevant PRs for Field Kit and farmOS 2.x.

Definitely!

Just a couple of notes I made while reviewing: [...]

Will fix those.

Would it make sense to add an agenda item to discuss these changes and next steps on the 5/12 monthly call (this Wednesday)? I don't think we have anything else planned.

Or, if we want to find a separate meeting time that works too. Would love to have both @paul121 and @jgaehring join.

I'm open to either or both. If I don't hear a preference for a stand-alone meeting from @paul121 and @jgaehring here, I'll add it this evening to the Monthly Call agenda.

[symbioquine]

On a related note, in the last 36 hours or so I've been experimenting with a strategy that would allow us to have our cake and eat it too! Specifically, we can somewhat avoid the costs of loading all of OpenLayers while still having it all available to contrib modules.

The key to doing that is by creating an intermediate package which leverages a new feature provided by Webpack called Module Federation - kind of like dynamic linking. Naively using that feature would yield one huge chunk. However, with a bit of scripting, the intermediate package can expose each importable package from OpenLayers as a separate "module" in the resulting federated "container".

In short, this will allow dynamic/lazy loading of individual imports from OpenLayers as cachable chunks.

Of course, nothing is free and the cost to the strategy described above is that we'd be trading increased network requests and packaging complexity for reduced bandwidth. I think this may be a good tradeoff especially since one of the main contributors to OpenLayers has indicated that the all-in-one builds of OpenLayers are "legacy" and "[they] discourage its use". As such, it may be better to incur the cost of a breaking change now, moving to this module federation strategy where we would control the packaging than to have the 2.x contrib ecosystem built against something which OpenLayers considers legacy and discouraged.

[jgaehring]

Awesome! I haven't had a chance to get caught up on this thread but eager to talk with y'all about it. Standalone meeting or monthly call is fine for me.

[symbioquine]

Sorry for the radio silence here. I've actually been putting in quite a bit of time on this, but I didn't have anything to report back until now...

I have been evaluating the Webpack Module Federation feature I mentioned above to determine whether it is worth the cost of increased packaging complexity.

BLUF¹

Module Federation is better than using the official legacy ol.js build.

For the examples tested, Module Federation can result in about 45% less JS data transferred over the network and 60% less JS data loaded client side compared with the official legacy ol.js build. That comes out to about 30% (~900ms) faster loading times in our "DSL network constrained" scenario.

Using module federation to package all of OpenLayers is (as expected) slower than the control case of including only the bits we need. For the map tested, this works out to about 45% (~640ms) longer loading times than the control in our "DSL network constrained" scenario.

Those loading time percentages above remain pretty consistent at different network speeds/latencies, but it is important to note that at 2G networks speeds, Module Federation could turn a 10s uncached page load into a 7s one - but is still worse than the 5s page load without including all of OpenLayers.

Details

I created a test bed to compare the performance of various packaging strategies; https://github.com/symbioquine/ol-federated/tree/main/examples

All examples render the same map scene - a stretch of OSM ocean with a LineString (code here);

I ran this performance test against each packaging strategy 10 times. The performance test gathers metrics for 5 scenarios (Unthrottled, Regular2G, Regular4G, DSL, SlowCPU) by loading the map twice - to get the uncached and cached behavior.

The packaging strategies differ only in their dependencies and webpack.config.js files;

• with-simple-webpack-bundling: Control case demonstrating best case performance with simple Webpack bundling (does not include all of OpenLayers)
• with-ol-externalized-to-static-ol-dot-js: Uses the official legacy ol.js build
• with-webpack-naive-federation: Uses a naive strategy to employ Module Federation and defeat tree shaking for OpenLayers
• with-webpack-naive-federation-fewer-chunks: Same as the previous one, but with optimization.splitChunks.{maxAsyncRequests, maxInitialRequests}=15
• with-ol-federated: My attempt to be clever by splitting every OpenLayers package as a separate Module Federation exposed module

For the given map scene those scenarios yield the following amount of JS sent over the network;

Also, here's the breakdown of the number of JS resources involved in each;

And the total bytes decoded/decompressed;

The most important metric (I've found so far) is what I'm calling nav-to-net-idle which captures the time it takes between the start of navigating to the new page and when the last resources have finished loading. The test captures the screenshot above immediately after that - demonstrating that is the point is when the map is fully rendered. (If perhaps not fully interactive.)

Zooming in on the DSL-uncached scenario;

We can see that the static ol.js case (3s) takes almost twice as long as the ideal/control case (1.5s). We also see that the naive Webpack Federation strategy is on par with my attempt at more fine-grained federation.

Zooming in on the SlowCPU scenarios;

We can see that Module Federation out-performs even the control case. This probably warrants further investigation, but my guess would be that the browser is able to better parallelize loading with the increased chunk count. This may be suggestive of the expected performance on modern mobile devices that have more (slower) cores.

---

I also measured the time to first-contentful-paint, but discovered that it was a less representative metric than nav-to-net-idle since it occurs sooner - and before the map is fully rendered.

---

first-contentful-paint.csv js-resource-count.csv js-resource-total-decoded-bytes.csv js-resource-total-transfer-bytes.csv nav-to-net-idle.csv 2021_05_13_profiling_data_summarized.json.zip

[symbioquine]

I've reviewed all the proposed changes in #109 and they look really good @symbioquine ! I like where this is headed!

Perhaps it makes sense to organize/summarize the "next steps" (maybe in the PR description?) so we can be sure everything is accounted for... including relevant PRs for Field Kit and farmOS 2.x.

Just a couple of notes I made while reviewing:

* We should include some instructions for updating from 1.x to 2.x, for anyone who's using farmOS-map v1 in their projects currently. I know there are a few out there aside from farmOS and Field Kit.

* Minor nitpicks / coding standards:

  * Changelog items should end in periods.
  * There are some extra newlines in README.md.
  * Needs a newline above comment inside `attachBehavior()`.
  * I haven't fully grokked the behavior sort logic - I wonder if we could add some comments, and maybe move it out to an include file? I like that `main.js` is so small/readable otherwise.

I've addressed all these things - except for the "update instructions" part which is included in the new "Known Remaining Work" section of the PR description.

The next step is I need alignment on whether it makes sense to pursue the module federation strategy instead of externalizing to the legacy static ol.js OpenLayers dist. @mstenta / @jgaehring / @paul121 Let me know what additional data/answers you'd need to make that decision. I'm happy to have a meeting/call about it too if that would help...