Move apps and team page scripts out of landing-page.js #27137
Conversation
|
This seems like good progress, but see the note above on |
amanagr
force-pushed
the
portico_refactor
branch
from
dfaa6b9 to
65a6ba3
Compare
amanagr
added
the
integration review
This helps us run the script independent of the URL path.
This is to make tests for channel work. Channel mocks `$` to be an object, so it cannot be called as an function in sentry.ts if it is imported in channel.ts. So, to avoid it, we extract the functions which channel.ts uses and fortunately they don't use `$`, so it works out.
amanagr
force-pushed
the
portico_refactor
branch
from
65a6ba3 to
2ae4e5c
Compare
| @@ -10,7 +10,8 @@ import * as Sentry from "@sentry/browser"; | |||
| import $ from "jquery"; | |||
|
|
|||
| import {BlueslipError, display_stacktrace} from "./blueslip_stacktrace"; | |||
| import {page_params} from "./page_params"; | |||
|
|
|||
| const development_environment = $("#page-params").data("params").development_environment; | |||
There was a problem hiding this comment.
This change seems dangerous, in that page_params.ts removes it from the DOM in the web app itself. Also, we really really don't want to have jQuery parse the params twice; it is an expensive operation for large organizations like chat.zulip.org.
Probably a good #frontend conversation about what we want to do with this detail.
There was a problem hiding this comment.
I think the most likely theory for how to fix this is to put the actual app parameters into a separate data-app-params bucket from the "common" stuff used by Sentry/Blueslip and smaller pages, and those things can be parsed multiple times safely as they would not be huge. But I'm not sure and it deserves a proper discussion.
|
Heads up @amanagr, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
|
So the |
Follow up on #27079 (comment)