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

1757 tm new routing #342

Closed
wants to merge 20 commits into from
Closed

1757 tm new routing #342

wants to merge 20 commits into from

Conversation

soyarsauce
Copy link
Contributor

@soyarsauce soyarsauce commented Jun 12, 2019

See: TerriaJS/terriajs#3494

Still to do

  • Create an "app-loaded event" to listen for when generating the html instead of a fixed timeout
  • Generate sitemap.xml with routes generated from generate-init-routes.js
  • Fix prettier PR @ Prettier apply #333

Also see:
TerriaJS/terriajs-server#106

@AnaBelgun AnaBelgun requested a review from kring June 24, 2019 00:44
@soyarsauce soyarsauce marked this pull request as ready for review June 26, 2019 01:00
@soyarsauce
Copy link
Contributor Author

soyarsauce commented Jun 26, 2019

OK @kring this is ready now, I don't know if we want travis to attempt prerendering as part of the build or just skip it for CI:

  • on one hand it's actually a good way for CI to catch if the build actually serves up an app that doesn't error out (e.g. it will wait forever if broken, which is what happened while I was tinkering with it and didn't realise I was building a JS-broken-build)
  • & on the other hand it will make builds long for big catalog items, or even bail out as it doesn't render any logging out while it's running through all the routes

Working CI test can be seen @ #349 - the only material difference is pointing to the right terriajs commit in package.json
(The timeout param didn't seem to have an effect)

@@ -1,10 +1,11 @@
<!DOCTYPE html>
<html lang="en" class="terria">
<head>
<base href="/">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually now index.html gets updated as part of the render-datasource-templates gulp task, should we remove this from source to avoid confusion?

@AnaBelgun
Copy link
Member

to redev/review after mobx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants