search

oscarfonts / mapbox-gl-cordova-offline

Offline vector maps in Cordova using Mapbox GL JS
Other
60 stars 36 forks source link

Updated mapbox-gl-js version to 1.11.0 #54

Open itbeyond opened 4 years ago

itbeyond commented 4 years ago

Also updated to support Cordova 9.0.0, Android 8.1.0. iOS 5.1.1

alistairfoggin commented 4 years ago

Having looked at the dependencies, there is still cordova-plugin-compat which is now deprecated as it is included in cordova-android. I believe that this dependency can now be removed along with upgrading the cordova-android and cordova-ios plugins. If there is still a need for it, then it can stay. I just wanted to check the necessity for cordova-plugin-compat before merging. Otherwise, this looks like a great pull request for this repository!

itbeyond commented 4 years ago

Compat can go away and the below could also be updated to the latest:

"cordova-plugin-device": "2.0.3",
"cordova-plugin-file": "6.0.2",

I am still working on another problem that I am struggling with. I switch styles in code using setStyle() and there is a validator that is running past the latest v8.json list of allowed style elements and crashing the style because of rasteroffline and/or mbtiles. I am trying to override this in the source but this is a little beyond my current scope of rollup and import inheritance. Can you point me in the right direction to add an updated v8.json file into the build pipeline that replaces the one at https://github.com/mapbox/mapbox-gl-js/blob/master/src/style-spec/reference/v8.json

I have written the new file but cannot work out how to get it into the final output.

alistairfoggin commented 4 years ago

Having looked closer at the code, there are rollup plugins that are used to transform code as it goes through the build process. These plugins are in build/rollup_plugins.js in the function plugins() on line 19. The replacement of the file will probably need to be before the minifyStyleSpec plugin. There is a plugin called rollup-plugin-re which allows you to replace files. It hasn't had any activity for 2 years but it should still work. The github page is here: https://github.com/jetiny/rollup-plugin-re It has an include option which is the file name that you want to edit (v8.json). Then in the patterns array, you have an object which has a key "file". The value is the path relative to the root of the repository. It will then replace contents of the input file with the contents of your file. At least that is what I understand it to do. I hope this helps. I haven't worked that much with rollup myself.

Also, one minor detail is that the dev dependencies in the package.json seem to be older versions of rollup and the rollup plugins. These may not need to be updated as they are not part of the final build. The official rollup plugins have moved from rollup-plugin- to @rollup/plugin- as shown here: https://github.com/rollup/plugins

Hopefully it isn't too difficult to get the style.json to work!

itbeyond commented 4 years ago

Thanks will investigate – I have built a full working copy (via edit of rollup output to include the needed updates) of the offline mapping system and plugged it into my active development system and it works really well and fast. I am rendering the offline tiles (now on desktop browser, browser, uwp, iOS and Android) against 30 odd other data layers and can switch between different base styles perfectly. I am just going to have to solve a weird thing with the offline style & font downloads and will try to get rollup to play with the required updates. Once I solve it, I will update or create a new pull. At least the pull #54 already provided will make the sample tiles and app work correctly – the issues I am still working on only present when you extend.

eduboxgithub commented 4 years ago

Thank you so much for your work.

Can this be merged to master ou release as a new version as when fixed?

A small question... With this update can I use Cordova 8.1.2 or I must use version 9 only?

itbeyond commented 4 years ago

This can be merged and it will work with the sample app without error. As for 8.1.2 I am fairly confident it will work I did push up the version of the sqlite plugin to get past a different error and not sure of it's requrements. The 9.0.0 update is not too bad once you sort out the q = require in your scripts btw.