Provide methods to transform coordinates and extents

[paul121]

Closes #171

I also included a simple getCurrentViewExtentCoordinates(source = 'EPSG:3857', destination = 'EPSG:4326') helper function. This is only for convenience and is not necessary to include here if transform and transformExtent are exported with farmOS-map. But it's a simple bit of code and seems like something that may be useful to others. Right now I'm bundling OL and providing this method myself: https://github.com/Regen-Digital/farm_farmlab/blob/1.0.0-beta2/js/farmos-map-extent/main.js#L5-L16

[paul121]

Little to no size increase if I'm reading this correctly. Seems reasonable that transform and transformExtent are already included one way or another.

Build before:

$ npm run build

> @farmos.org/[email protected] build
> npm run lint && webpack --config webpack.config.js --mode production


> @farmos.org/[email protected] lint
> eslint src && stylelint 'src/**/*.css'

assets by status 89.3 KiB [cached] 20 assets
assets by path . 463 KiB
  assets by path *.js 442 KiB
    asset farmOS-map.js 429 KiB [emitted] [minimized] [big] (name: main) 1 related asset
    asset mapbox.js 12.7 KiB [compared for emit] [from: examples/simple-html-consumer/static/mapbox.js] [copied] [minimized]
  asset farmOS-map.css 20 KiB [compared for emit] (name: main)
  asset index.html 1.42 KiB [compared for emit] [from: examples/simple-html-consumer/static/index.html] [copied]
Entrypoint main [big] 449 KiB = farmOS-map.css 20 KiB farmOS-map.js 429 KiB
orphan modules 1.07 MiB [orphan] 141 modules
runtime modules 10.1 KiB 12 modules
code generated modules 1.87 MiB (javascript) 30.9 KiB (css/mini-extract) [code generated]
  modules by path ./node_modules/ol/ 1.12 MiB (javascript) 5.08 KiB (css/mini-extract) 107 modules
  modules by path ./src/ 694 KiB (javascript) 4.99 KiB (css/mini-extract) 26 modules
  modules by path ./node_modules/ol-layerswitcher/dist/ 26.6 KiB (javascript) 5.06 KiB (css/mini-extract) 2 modules
  modules by path ./node_modules/ol-geocoder/dist/ 16.8 KiB (javascript) 6.99 KiB (css/mini-extract) 2 modules
  modules by path ./node_modules/ol-popup/ 6.18 KiB (javascript) 1.09 KiB (css/mini-extract) 2 modules
  modules by path ./node_modules/ol-side-panel/ 7.65 KiB
    css ./node_modules/css-loader/dist/cjs.js!./node_modules/ol-side-panel/dist/ol-side-panel.css 3.82 KiB [code generated]
    css ./node_modules/css-loader/dist/cjs.js!./node_modules/ol-side-panel/src/SidePanel.css 3.82 KiB [code generated]
  ./node_modules/bootstrap-icons/icons/layers-half.svg 386 bytes [built] [code generated]
  ./node_modules/rbush/rbush.min.js 6.31 KiB [built] [code generated]
  ./node_modules/ol-grid/dist/ol-grid.cjs.js 14.6 KiB [built] [code generated]
webpack 5.38.1 compiled successfully in 5002 ms

Build after:

$ npm run build

> @farmos.org/[email protected] build
> npm run lint && webpack --config webpack.config.js --mode production


> @farmos.org/[email protected] lint
> eslint src && stylelint 'src/**/*.css'

assets by status 89.3 KiB [cached] 20 assets
assets by status 463 KiB [compared for emit]
  assets by path *.js 442 KiB
    asset farmOS-map.js 429 KiB [compared for emit] [minimized] [big] (name: main) 1 related asset
    asset mapbox.js 12.7 KiB [compared for emit] [from: examples/simple-html-consumer/static/mapbox.js] [copied] [minimized]
  asset farmOS-map.css 20 KiB [compared for emit] (name: main)
  asset index.html 1.42 KiB [compared for emit] [from: examples/simple-html-consumer/static/index.html] [copied]
Entrypoint main [big] 449 KiB = farmOS-map.css 20 KiB farmOS-map.js 429 KiB
orphan modules 1.07 MiB [orphan] 142 modules
runtime modules 10.1 KiB 12 modules
code generated modules 1.87 MiB (javascript) 30.9 KiB (css/mini-extract) [code generated]
  modules by path ./node_modules/ol/ 1.12 MiB (javascript) 5.08 KiB (css/mini-extract) 107 modules
  modules by path ./src/ 695 KiB (javascript) 4.99 KiB (css/mini-extract) 26 modules
  modules by path ./node_modules/ol-layerswitcher/dist/ 26.6 KiB (javascript) 5.06 KiB (css/mini-extract) 2 modules
  modules by path ./node_modules/ol-geocoder/dist/ 16.8 KiB (javascript) 6.99 KiB (css/mini-extract) 2 modules
  modules by path ./node_modules/ol-popup/ 6.18 KiB (javascript) 1.09 KiB (css/mini-extract) 2 modules
  modules by path ./node_modules/ol-side-panel/ 7.65 KiB
    css ./node_modules/css-loader/dist/cjs.js!./node_modules/ol-side-panel/dist/ol-side-panel.css 3.82 KiB [code generated]
    css ./node_modules/css-loader/dist/cjs.js!./node_modules/ol-side-panel/src/SidePanel.css 3.82 KiB [code generated]
  ./node_modules/bootstrap-icons/icons/layers-half.svg 386 bytes [built] [code generated]
  ./node_modules/rbush/rbush.min.js 6.31 KiB [built] [code generated]
  ./node_modules/ol-grid/dist/ol-grid.cjs.js 14.6 KiB [built] [code generated]
webpack 5.38.1 compiled successfully in 4784 ms

[mstenta]

This LGTM. The only small thought I have is: should we create some kind of separation between "instance methods" that we provide and functions provided by OpenLayers itself?

Specifically, it feels a bit weird to me that we will have instance.tranform() and instance.transformExtent() instance methods. The way they read is a bit misleading because it feels like it should mean "transforming the instance" - but they aren't actually doing anything to the instance in question. The instance.getCurrentViewExtentCoordinates() method does make intuitive sense in this regard, because it's acting on this.map.

OpenLayers itself makes it clear that these are generic helper functions (not specific to any map instance) by putting them in ol/proj. Should we do something like that instead?

@paul121 you mentioned something similar to this in your original #171 comment:

Could we either export this function and make it available at window.farmOS.map or make it available on the farmOS-map instance itself?

This sort of gets at the bigger question/issue we have which is: how do we make general-purpose OL functions available to others without packaging all of OL. Maybe this is an opportunity to start a pattern?

Would something like window.farmOS.map.ol make sense as a new global bucket to put these in?

Then we could have:

• window.farmOS.map.ol.proj.transform()
• window.farmOS.map.ol.proj.transformExtent()
• ... others as we decide to expose them?

[mstenta]

Would something like window.farmOS.map.ol make sense as a new global bucket to put these in?

Just thinking more broadly about this... imagine if we started to provide some additional packaged JS files, alongside farmOS-map.js, which basically just add different sets of OL functions/objects to the window.farmOS.map.ol namespace.

Within farmOS itself (Drupal), we could represent each as a separate JS library, so they could be included separately as needed.

For example, say someone wants to use ol.format.KML in a behavior. Perhaps farm_map.libraries.yml could including something like this:

farmOS-map-ol-format-kml:
  js:
    /libraries/farmOS-map/farmOS-map-ol-format-kml.js:
  dependencies:
    - farm_map/farmOS-map

And then my custom behavior could depend on that:

behavior_mykmlstuff:
  js:
    js/farmOS.map.behaviors.mykmlstuff.js: { }
  dependencies:
    - farm_map/farm_map
    - farm_map/farmOS-map-ol-format-kml

Probably deserves a separate thread to discuss this stuff... just dropping the idea here to start. :-)

[paul121]

Specifically, it feels a bit weird to me that we will have instance.tranform() and instance.transformExtent() instance methods.

Would something like window.farmOS.map.ol make sense as a new global bucket to put these in?

Yeah I agree this is odd. I had thought about a more generic util namespace too.. but yes, it probably makes sense to keep the ol patterns.

Within farmOS itself (Drupal), we could represent each as a separate JS library, so they could be included separately as needed.

I'd be happy with that, as long as there's a way for JS apps to leverage this too? Would they just include the separate JS files as needed on a page? Alternatively I wonder if we could use the webpack bundling + dynamic imports for this.. I know our behaviors can specify what dependencies they need, but not sure if we could use that same thing for this.

[mstenta]

I'd be happy with that, as long as there's a way for JS apps to leverage this too? Would they just include the separate JS files as needed on a page? Alternatively I wonder if we could use the webpack bundling + dynamic imports for this.. I know our behaviors can specify what dependencies they need, but not sure if we could use that same thing for this.

I think that would work.

It's not really an issue for any JS apps that use webpack bundling - they already dynamically import only the bits they need (including farmOS-map).

So it would primarily support the apps that don't use JS bundling. farmOS itself basically falls into this category, and can only add pre-built JS files to the page. So if we had some pre-built "extra" files that simply added functions/objects to window.farmOS.map.ol then it feels like it would open up some possibilities, without bloating the pre-built farmOS-map.js file itself.

[symbioquine]

Thanks for working on this @paul121!

I'm also still fairly dubious about exposing generic OL methods as part of the farmOS-map API. That's why I spent so long looking into https://github.com/farmOS/farmOS-map/issues/68

I definitely, recognize the need for something like this, but am still concerned about tacking them on piecemeal to our API. In my opinion, it should be structured under some sort of namespace that mirrors the OL imports/name-spacing (for obvious documentation/maintenance reasons) - and should be a subset of OL. (...or all of it, if we can figure out a federation/chunking strategy that works out efficiently.)

As a path forward without figuring that stuff out, I would recommend we create a method farmOS.map.unstableImportOL(importPath) which is hard-coded to return the specific bits we're exposing for specific input strings - like 'ol/proj/transform'. That makes calls to the method somewhat self-documenting, makes the structure mostly mirror the structure of OL imports, and gives us a place to put a deprecation warning once we figure out a better strategy.

[paul121]

I would recommend we create a method farmOS.map.unstableImportOL(importPath)

Thanks @symbioquine, I like this idea. I'm not 100% sure how this would work... but it seems like a simple approach would still add all of the imports to the farmOS-map.js entrypoint. Do you think this function could delegate to load separate chunks on-demand? Maybe we build chunks based on the path prefix eg: ol/proj, ol/extent?

Brings me back to your Webpack Module Federation ideas too and preventing common OL dependencies from being multiple times (tl;dr at end of this comment: https://github.com/farmOS/farmOS-map/issues/68#issuecomment-851043516) specificially:

Not adopt Webpack Module Federation for OpenLayers at this time - this seems elegant, but in practice the tooling isn't there yet to intelligently tune the chunks sizes for a large dependency like OpenLayers

It has been nearly 2 years since then... do you think this is still the case? ;-)

imagine if we started to provide some additional packaged JS files, alongside farmOS-map.js, which basically just add different sets of OL functions/objects to the window.farmOS.map.ol namespace.

Although coming back to @mstenta idea purely from the Drupal perspective.. this could even be provided with a library entirely separate from farmOS-map. If we didn't want to clutter farmOS-map.js with this bloat. We could build these in a separate library just for drupal.

So now I just wonder... if I should just start bundling more & more of my JS behaviors. It seems like we will always be limited by not bundling. And if there aren't many performance improvements in we can add to farmOS-map.js with Module Federation or otherwise... maybe that is the better "recommended" solution in the end?

[symbioquine]

Thanks @symbioquine, I like this idea. I'm not 100% sure how this would work... but it seems like a simple approach would still add all of the imports to the farmOS-map.js entrypoint. Do you think this function could delegate to load separate chunks on-demand? Maybe we build chunks based on the path prefix eg: ol/proj, ol/extent?

Definitely. I'll reply with more detail about how that could work in the morning...

It has been nearly 2 years since then... do you think this is still the case? ;-)

No idea. I hope that the tooling has matured very much since then!