terriajs icon indicating copy to clipboard operation
terriajs copied to clipboard

Experiment to cleanup Terria start, and init sources

Open nf-s opened this issue 2 years ago • 2 comments

Cleanup Terria start, and init sources

Fixes #6354

Main purpose of this PR is to simplify Terria.ts, Terria.start and InitSources.

Previously loadInitSources was getting called multiple times - at different stages of start and TerriaMap/index.js.

Now loadInitSources must be called explicitly after Terria.start()

I have also split up Terria.ts into

TerriaConfig.ts

  • All interfaces - TerriaConfig, ConfigParameters, StartOptions, Analytics, HomeCameraInit

InitSource.ts

now handles all adding of init sources - none of these new functions call loadInitSources

  • setupInitializationUrls -> addInitSourcesFromConfig
  • interpretStartData -> addInitSourcesFromStartData
  • updateApplicationUrl -> addInitSourcesFromUrl
  • generateInitializationUrl -> generateInitializationUrl (private function)
  • interpretHash -> interpretHash (private function)

As I have removed the following functions in Terria:

  • updateFromStartData - instead use addInitSourcesFromStartData and then call terria.loadInitSources
  • updateApplicationUrl - instead use addInitSourcesFromUrl and then call terria.loadInitSources

InitData.ts

now handles all applying of init data

  • terria.applyInitData -> applyInitData
  • terria.loadModelStratum -> loadModelStratum (private function)
  • terria.pushAndLoadMapItems -> pushAndLoadMapItems (private function)

PickedFeatures.ts

  • loadPickedFeatures -> loadPickedFeaturesFromJson in PickedFeatures.ts

Changes required to index.js in TerriaMap

See PR https://github.com/TerriaJS/TerriaMap/pull/603 for new load procedure:

  • terria.start (await)
  • Load plugins
  • terria.loadInitSources (async)
  • ... TerriaMap specifics - eg disclaimer
  • render

https://github.com/TerriaJS/TerriaMap/blob/218633b1171b87f2f3c0520480c7a7ce24fabcb7/index.js#L62-L85

I have removed all Magda config functionality from Terria - this will need to be re-implemented in TerriaMap

Removed code is here https://gist.github.com/nf-s/4381b585fb2115a6e4d35dbcf77909c5

Other changes

  • TSify updateApplicationOnMessageFromParentWindow
  • updateApplicationOnHashChange and updateApplicationOnMessageFromParentWindow are now called in Terria.start
    • Added disableUpdateApplicationOnHashChange and disableUpdateApplicationOnMessageFromParentWindow to StartOptions
  • Added getConfig to Terria StartOptions - this can be used to override fetching of Terria config through StartOptions.configUrl
  • Terria.start will now await Internationalization init

Questions

Plugins

  • What hooks are required?

updateApplicationOnHashChange and updateApplicationOnMessageFromParentWindow

  • are they ever called on non-global window object?
  • do we have maps where they aren't called?

Test links

Start data with huge GeoJSON file

Checklist

  • [x] There are unit tests to verify my changes are correct or unit tests aren't applicable (if so, write quick reason why unit tests don't exist)
  • [ ] I've updated relevant documentation in doc/.
  • [x] I've updated CHANGES.md with what I changed.
  • [ ] I've provided instructions in the PR description on how to test this PR.

nf-s avatar Jun 29 '22 15:06 nf-s

without fixing this, it affects Stories loading for example (i.e. they might be slow to load due to GeoJSON files)

AnaBelgun avatar Aug 03 '22 04:08 AnaBelgun

How does this interact with plugin lifecycle?

steve9164 avatar Nov 09 '22 04:11 steve9164