-
-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide methods to transform coordinates and extents #189
base: 2.x
Are you sure you want to change the base?
Conversation
Little to no size increase if I'm reading this correctly. Seems reasonable that 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 |
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 OpenLayers itself makes it clear that these are generic helper functions (not specific to any map instance) by putting them in @paul121 you mentioned something similar to this in your original #171 comment:
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 Then we could have:
|
Just thinking more broadly about this... imagine if we started to provide some additional packaged JS files, alongside 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
And then my custom behavior could depend on that:
Probably deserves a separate thread to discuss this stuff... just dropping the idea here to start. :-) |
Yeah I agree this is odd. I had thought about a more generic
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 |
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 #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 |
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 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: #68 (comment)) specificially:
It has been nearly 2 years since then... do you think this is still the case? ;-)
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? |
Definitely. I'll reply with more detail about how that could work in the morning...
No idea. I hope that the tooling has matured very much since then! |
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 iftransform
andtransformExtent
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