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

Enabling redux devtools in production #465

Merged
merged 3 commits into from
Sep 13, 2022
Merged

Enabling redux devtools in production #465

merged 3 commits into from
Sep 13, 2022

Conversation

olemartinorg
Copy link
Contributor

@olemartinorg olemartinorg commented Sep 13, 2022

Description

This change enables redux devtools in the production build, which brings with a few hundred bytes extra code, as well as a 7.1mb altinn-app-frontend.js.map and a 556kb altinn-app-frontend.css.map to be published on altinncdn.no.

Adding these to the production build gives us a few advantages when debugging and reproducing strange bugs:

  • We can record the redux state history, saving them to a file, making it possible to save and replay the state history for race condition bugs
  • Thrown exceptions in our own code will include references to the actual source files, lines and code that caused the error (instead of line 1, column 489984)

Related Issue(s)

Verification

  • Your code builds clean without any errors or warnings
  • Manual testing done (required)
  • [ ] Relevant automated test added (if you find this hard, leave it and we'll help out)
  • All tests run green

Documentation

@olemartinorg olemartinorg marked this pull request as draft September 13, 2022 10:48
@olemartinorg olemartinorg marked this pull request as ready for review September 13, 2022 11:59
@olemartinorg
Copy link
Contributor Author

I spoke with @lorang92 and @altinnadmin about this, and we decided against the 7mb source map this time around. Having it checked into the git repo for cdn, hosted on github spaces, etc, would quickly balloon into much more disk usage than just 7mb.

We also considered:

  • Hosting the source maps elsewhere
  • Only copying over the source map for the major version to CDN (skipping source maps inside every minor/patch folder)
  • Gzipping (1.8mb) or compressing with brotli (1.4mb), however it seems devtools are not ready for a .js.map.gz or .js.map.br yet - it didn't load correctly (might have been an issue with the wrong HTTP response headers being generated by my static file web server).

TL;DR: Skipping source maps for now

@olemartinorg olemartinorg changed the title Enabling redux devtools and source maps in production Enabling redux devtools in production Sep 13, 2022
@@ -22,7 +22,7 @@ export const setupStore = (preloadedState?: PreloadedState<RootState>) => {

const innerStore = configureStore({
reducer: reducers,
devTools: isDev,
devTools: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a note: this defaults to true, so strictly not needed config. But better to be explicit I guess!

Neat 🥇

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, didn't know that! But yeah, I don't mind that being explicit.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ivarne
Copy link
Member

ivarne commented Sep 13, 2022

How expensive are those sourcemaps really? They won't be downloaded unless people open devtools. With the current state of react frontend crashing for every error in the json files, it would really help to speed up our debugging process to figure out what is wrong.

@olemartinorg
Copy link
Contributor Author

@ivarne I agree they would be useful, especially for more experienced developers, but as of now they might be problematic to add to the CDN. As the CDN is backed by a git repo, every new file needs to be checked into git, grows the history, and might affect our ability to serve the CDN at all (github static pages have storage limitations). I'm hoping for this to easier in the future, possibly after moving the CDN to azure. Right now, the risks outweighs the benefits, IMO.

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