ESLint + debug ElasticAPM RUM #504
Conversation
This bug was missed in testing.
This should simplify the look of the user interface. TODO: add virtual sources display?
This allows an AmpliPi to be shallowly rebranded in the web app.
This allows a different picture to be displayed while waiting for a connection
|
The main functional changes added in my commits would be just commenting out unused variables, so if there's something wrong I'd suggest searching for comment delineators, especially in the earlier commits |
|
Why do we want "top level await" in particular? |
| try { | ||
| const r = await fetch("/debug"); | ||
| const debugDocText = await r.text(); | ||
| const debugDoc = JSON.parse(debugDocText); | ||
| const debug = (debugDoc && debugDoc["debug"]) ? true : false; | ||
|
|
||
| var apm = initApm({ // eslint-disable-line no-unused-vars | ||
| active: debug, | ||
| serviceName: "Amplipi", | ||
| serverUrl: debugDoc["apmHost"] ? debugDoc["apmHost"] : "", |
There was a problem hiding this comment.
@jonahshader I use top level await here to instantiate a very early connection to an arbitrary APM server, specified from a debug endpoint. I could also resolve the promise myself, or use a fancy vite plugin to transform top level awaits into resolved ones. I just didn't see a reason not to track more modern browsers and it seemed least messy at the time.
There was a problem hiding this comment.
Ah, got it. I think @linknum23 should answer this since he advocated for more backwards compatibility in the past.
There was a problem hiding this comment.
for context, introducing top level await drops support from estimates of 95% to 90% of global web browsers; most anything released since 2021 has this support. All long term support (read: oldest supported) versions of Firefox and Chrome support es2022.
on a super opinionated note, I'm not opposed to using browser compatibility as a forcing function to get folks to upgrade. Browsers have the most pronounced attack surface of any commonly used software. The equitability/accessibility concerns are minimal here, outweighed by serious security considerations.
|
Looking at the react static propType, react just recommends using TypeScript instead. How difficult would it be to turn this into a TypeScript project? Considering that TypeScript is a strict superset of JavaScript, making that change won't break anything, and we could slowly add type checking to our components as needed. We could also add type checks to our other non-component functions, which isn't supported by prop-types. |
This is a wombo-combo of two distinct things - the work @SteveMicroNova did to ESLint the entire repo, and the ElasticAPM integration for a quick debug mode config to get feedback on
webapp_2. This is the branch that's currently playing things in the office 😎 I'm opting to PR this intowebapp_2so we can begin working on merging #494 🙃 It might make sense to review this PR commit by commit, and skipping the large linting commits; should some concerns arise about some of the super buried bits of this, we can re-constitute a branch with cherry-picked commits and work off that.Things I'd appreciate feedback on:
es2022for top level await? I know that makes me a smidge lazy... but Slack is deprecating my Firefox 107 browser on Sept 1st so I think we're probably fine lol