build: Add script to build & minify loader #46780
Conversation
github-actions
bot
added
Scope: Frontend
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
| @@ -0,0 +1,233 @@ | |||
| ( | |||
| function sentryLoader( | |||
There was a problem hiding this comment.
Note: Without making this non-anonymous, terser keeps inlining __LOADER_IS_LAZY__ as a constant (which wouldn't be terrible, but it's nicer to have this all at the end). Adding a function name fixes this (and it is actually removed in the minified bundle anyhow).
scripts/build-js-loader.js
Outdated
There was a problem hiding this comment.
Are you able to write this script in typescript?
There was a problem hiding this comment.
agree with @evanpurkhiser ... and wouldn't it be nice to have prettier formatting this as well?
There was a problem hiding this comment.
So I set it up to have prettier & eslint. We could use typescript but that would complicate the build step - we'd need to use either rollup or babel or something like this then. Right now we just use the script as-is mostly (which also means the script has to be ES5 compatible!)
There was a problem hiding this comment.
We have ts-node installed, so it should just work
There was a problem hiding this comment.
OK, so i refactored it to use typescript and use typescript directly to compile it!
There was a problem hiding this comment.
I also rewrote the build-script itself to typescript.
|
will this generate the script we are displaying in the onboarding? it is not clear to me what is the goal of this PR |
This script generates the actual Django templates from a base .js file. The goal of this is mainly to streamline the minification of this script, as previously we did that manually e.g. online by closure compiler. This is a) manual, b) error prone, c) not reproducible very well (and actually lead to an incident last Friday). I added a comment to the top of the script explaining the purpose of the script in detail as well! Note: I've also checked the source script for ES5 compatibility to make sure it is valid ES5 JS. |
|
OK, so I tested this multiple times in a local test app, both minified and unminifed seem to be working as expected! |
This adds a simple build script for the JS Loader. This should help with two things:
This just has to be run manually whenever the loader script is changed, the output is fully checked in etc. so nothing changes there.
I've tried the minified output in a local test app, would be great if someone else could also give it a try!