Skip to content
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

Experiment to cleanup Terria start, and init sources #6366

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

nf-s
Copy link
Contributor

@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)

@steve9164
Copy link
Member

How does this interact with plugin lifecycle?

@nf-s nf-s marked this pull request as ready for review November 25, 2022 00:27
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
3 participants