Skip to content

Conversation

@nf-s
Copy link
Member

@nf-s nf-s commented Jun 29, 2022

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 TerriaJS/TerriaMap#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

  • 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/.
  • I've updated CHANGES.md with what I changed.
  • I've provided instructions in the PR description on how to test this PR.

@AnaBelgun
Copy link
Member

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

@tephenavies
Copy link
Contributor

How does this interact with plugin lifecycle?

@nf-s nf-s marked this pull request as ready for review November 25, 2022 00:27
@CLAassistant
Copy link

CLAassistant commented Nov 7, 2024

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix load order

5 participants