feat: mangle error codes to error indexes

[andrewmcgivery]

PR Type

Does this PR add a new feature, or fix a bug?

Feature (ish)

Why should this PR be included?

This small addition to the build script results in a savings of around 30% when minified/bundled.

Before/After of Redux.min.js:

Checklist

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Is there an existing issue for this PR?
  • Consider using error code strategy employed by react #3655
• Have the files been linted and formatted?
• Have the docs been updated to match the changes in the PR? (Need feedback)
• Have the tests been updated to match the changes in the PR? (Haven't added tests... looking for feedback if this is needed)
• Have you run the tests locally to confirm they pass?

New Features

What new capabilities does this PR add?

Added mangleErrors babel script which converts error messages from strings to error code indexes per #3655.

For example:

Before:

throw new Error("This is my error message.");
throw new Error("This is a second error message.");

After (with minify):

throw new Error(0);
throw new Error(1);

After (without minify):

throw new Error(node.process.NODE_ENV === 'production' ? 0 : "This is my error message.");
throw new Error(node.process.NODE_ENV === 'production' ? 1 : "This is a second error message.");

What docs changes are needed to explain this?

May need a new doc around debugging in production but hoping the documentation king @markerikson has some ideas here :)

Outstanding Questions

• Do we need a change in documentation to explain this behavior?
• Do the error codes need to be documented somewhere or can we just reference the errors.json file?
• Are we ok with errors.json being in the root directory or should I move this to the /scripts folder?
• Any concerns with the output of the errors.json file?
• It appears I may have been the first person into the rollup configuration file with prettier enabled which resulted in A LOT of formatting changes... is this a problem?
• Is this MR even valuable? I decided to take a crack at it out of interest and wanting to contribute back to redux but I won't be offended if the answer is "nah".

[netlify]

Deploy preview for redux-docs ready!

Built with commit 1cd8f07

https://deploy-preview-3920--redux-docs.netlify.app

[markerikson]

Hey, this is pretty neat! And yes, saving 2.1K is totally worth it.

First few questions off the top of my head:

• master is the develop branch for what will, at some point, be Redux 5.0, but we don't have any actual timeline on when that will be released. Is this something we can / should try porting into the v4.x line? If so, is that just a minor version bump?
• How does the React docs site handle the error decoder? Is that something that's Gatsby specific? Can it be set up to work with Docusaurus?
• I think TSDX may do something like this. Have we looked at how they handle it?
  • Side thought: is it worth looking at switching to TSDX for building Redux v5 itself?
• How does this work with varying versions over time? What happens if we add a new error message somewhere? Do we need to keep around each version's error messages list permanently? If so, where / how do we do that?
• Can we examine doing this for React-Redux and Redux Toolkit as well?

Also paging @phryneas and @msutkowski here

[andrewmcgivery]

@markerikson Awesome! Putting my 2 cents to your points but I assume the folks you tagged will have some thoughts too!

• It should be easily portable into 4.x... I can open a separate MR for that after we work out the kinks in this MR. I think it would be a minor bump since there technically isn't any breaking changes to the API itself, assuming we're going off semver here.
• I've been searching through React to see what they do and it seems like they do some magic to link to a page in their docs (https://reactjs.org/docs/error-decoder.html/, example with an error code passed: https://reactjs.org/docs/error-decoder.html/?invariant=1) (see: https://github.com/facebook/react/blob/75955bf1d7ff6c2c1f4052f4a84dd2ce6944c62e/packages/shared/formatProdErrorMessage.js). Based on their documentation "This file is generated by the Gulp plugin and is used by both the Babel plugin and the error decoder page in our documentation." I'm not completely sure how we could do that here with docusaurus... That would be a new one for me. 😄 (EDIT: Just looked at the code for the redux site, it's VERY custom Gatsby code)
  -Can't answer fully for TSDX but based on their documentation, there would be some refactoring required since "this extraction only works if your error checking/warning is done by a function called invariant", but yes, it does do the same thing as React and the same idea as this. So if you do switch to TSDX, you'll get it out of the box with some refactoring and this custom plugin won't be needed.
  -So the file itself is appended to whenever a NEW error message comes up, old indexes remain in place. My assumption is that the version tags would handle the difference between each version of this file but I may be oversimplifying your question.
  -Absolutely can look at React-Redux and Redux toolkit! Provided they are using a similar build setup with rollup/babel, should be more or less a plug and play. 😄

[andrewmcgivery]

So, at the risk of comment spam, I did a quick experiment with docusaurus and it is totally possible to do something with the error codes.... here's an example (excuse the lack of styling) if we just outputted all of them by enumerating over the generated json file:

I just created a small React component in website/src/pages/errors.js:

import React from 'react'
import Layout from '@theme/Layout'
import useDocusaurusContext from '@docusaurus/useDocusaurusContext'
import styles from './styles.module.css'
import errorCodes from '../../../errors.json'

function Errors() {
  const context = useDocusaurusContext()
  const { siteConfig = {} } = context

  return (
    <Layout
      title={`${siteConfig.title} - A predictable state container for JavaScript apps.`}
      description="A predictable state container for JavaScript apps."
    >
      <main className={styles.mainFull}>
        <h1>Production Error Codes</h1>
        <p>
          When Redux is built and running in production, error text is replaced
          by indexed error codes to save on bundle size. These error codes can
          be found below.
        </p>
        <table>
          <thead>
            <tr>
              <th>Error Code</th>
              <td>Message</td>
            </tr>
          </thead>
          <tbody>
            {Object.keys(errorCodes).map(code => {
              const message = errorCodes[code]

              return (
                <tr>
                  <td>{code}</td>
                  <td>{message}</td>
                </tr>
              )
            })}
          </tbody>
        </table>
      </main>
    </Layout>
  )
}

export default Errors

[markerikson]

Sweeeeet! Yeah, that looks like a good starting point. Seems like we ought to be able to build on that.

Someone on Twitter was digging into how React implements things on their side:

https://twitter.com/giovannibenussi/status/1320547654028431360

[andrewmcgivery]

Sweeeeet! Yeah, that looks like a good starting point. Seems like we ought to be able to build on that.

Someone on Twitter was digging into how React implements things on their side:

https://twitter.com/giovannibenussi/status/1320547654028431360

Yea, I was digging into that code earlier which is where I was saying "Just looked at the code for the redux site, it's VERY custom Gatsby code" in my previous comment. My stuff above with docusaurus is pretty similar to what they are doing.

I think we could do something similar of linking to a page on the docs site, just would be slightly less savings in code as we'd have to add a function that includes the message with the link. So, depends on what we think would be the "best" option for consumers.

[andrewmcgivery]

Ok, last comment of the night!

For the repos already using TSDX (Redux Toolkit for example), it's probably best to work with the build tool. This will require some refactoring to replace all throws with an invariant implementation. Seems like tiny-invariant (https://www.npmjs.com/package/tiny-invariant) would be a good choice to keep the bundle size small, or possibly keep a copy of an invariant function in each repo and strip it down even more?

In either case, errors would be refactored:

// Before
throw new Error("This is my error message.");

// After
invariant(false, "This is my error message.");

If we want to explore this in Redux Toolkit, I'm happy to add to my todo list and give it a shot sometime this week. :)

[phryneas]

Looks like a very welcome contribution to me.
As for the docusaurus mdx: Keep in mind that that mdx will be evaluated on runtime, it is not part of the SSR. I'm not sure how the json loading will happen - is it inlined or loaded on page load? And I'm not 100% sure how search engines will index that. Should not be a problem, but taking a second look might be a good idea.

[giovannibenussi]

Hi! I'm the guy from Twitter 😜
Basically for those not familiar with Gatsby, you can define at build time custom GraphQL queries. In this case, they retrieve data from https://raw.githubusercontent.com/facebook/react/master/scripts/error-codes/codes.json and shape it in the following way:

query ErrorPageMarkdown($slug: String!) {
    // other fields
    errorCodesJson {
      internal {
        contentDigest
      }
    }
  }

Basically you don't have to worry about data retrieval in the ErrorDecoder component

<ErrorDecoder
  errorCodesString={data.errorCodesJson.internal.contentDigest}
  location={location}
/>

However, this works well because they retrieve this data once per build and not in real time.

I'm not familiar with redux's docs nor your CI servers, but if you like react docs' solution, just out of my head I can think about copy the contents from errors.json directly from github to your docs' source code in a step previous to build the docs when deploying.

[msutkowski]

Looks good to me as well! Regarding possibly switching to TSDX, redux exports an ESM browser build which is currently a small challenge to support with TSDX as shown in this PR: reduxjs/redux-toolkit#658. This was also recently referenced in this TSDX issue - outside of that, when I opened that PR all of the custom rollup configs redux uses basically match the TSDX defaults, so it should be pretty straightforward.

[timdorr]

We started the TS conversion before TSDX really took off, so it wasn't in the cards. I'd love to switch to it, but obviously that's outside the scope of this PR.

I would like to incorporate this compressed error approach. However, it is technically a breaking change (for production builds, no less), so I don't think it's semver to do this in the 4.x branch. Let's keep this on master and a bonus for those upgrading.

[andrewmcgivery]

So then for next steps...

• Is the documentation page for this within the scope of this MR? We have a technical solution but obviously it needs some work. I could also add the skeleton as part of this MR and it could be refined in future MRs.
• Should we use the React approach to call a function that includes a link to the error page within the production error message instead of just the number?
• TSDX is NOT in scope for this MR and should be deferred to a future MR.
• In a separate MR, update error handling in Redux Toolkit (and look into React-Redux as well) to use invariant instead of throw. (assuming this will be considered a breaking change in those libraries as well so I'll need some guidance on what branching strategy to use)

@markerikson Thoughts? 😄

[andrewmcgivery]

Based on jaredpalmer/tsdx#759 (comment) and jaredpalmer/tsdx#912 (comment) there are some issues in TSDX (more specifically upstream in rollup) that will cause some issues in all redux repos. Seems like the error extraction isn't a well supported feature.

[markerikson]

@andrewmcgivery nice research.

I'd say let's see if we can put together some kind of complete solution in this PR, including generating an appropriate docs page based on a JSON file extracted earlier in the docs build step.

[andrewmcgivery]

@markerikson ok so some changes!

Using a shared function that is injected in dynamically just like the React implementation, error messages now output a message with a link to the site with an error code in the URL:

Minified Redux error #${code}; visit https://redux.js.org/Errors?code=${code} for the full message or use the non-minified dev environment for full errors.

Took some heavy inspiration from React's page and tried to make the styles match the docs page as much as possible, looks like this when you follow that URL:

Worth mentioning, this is slightly less savings due to the function but still savings:

Errors file is automatically updated during build process and the website pulls the error codes from it directly.

Anything missing? :)

EDIT: Seems to be some build issues with the website. Working through that.

Edit 2: Fixed 😄

Edit 3: Apparently the latest VSCode update broke prettier so I had to run the formatter cause it was breaking the build. Fingers crossed it'll pass this time!

[andrewmcgivery]

@markerikson Sorry, I know you're busy with the doc rewrites and I don't want to be that guy that bumps a PR... I just don't know the proper protocol to ping you. 😨

[markerikson]

Yeah, thanks for the ping, actually - I missed the last update here.

A couple more questions:

• Just for kicks, can we throw in the rest of the error codes on this page in case anyone wants to reference them?
• Does this work right with the UMD prod build?

Personally, I still think we ought to be able to do this for 4.x. @timdorr , I wouldn't see this as a "breaking" change per se, although I can understand why it could be viewed that way. Also, given that we have no concrete plans to release 5.0 any time soon, I'd see this as a net win - shrinking some byte size off the core lib. Say, 4.1.0?

[andrewmcgivery]

@markerikson Added the full list of codes to the page. Screenshots of light and dark themes:

Regarding the UMD prod build, is there some way you'd prefer to verify this? I took a quick look through the minified source code and I do see the minified formatProdErrorMessage function and it being used throughout the code with the error codes instead of the text.

[markerikson]

I could pull and build this myself, but don't feel like doing so atm :) I'll take your word for it for now.

FWIW, I threw the question out on Twitter and over in Reactiflux. Some responses:

I would do that in a major bump I think - it's possible people are relying on the error message format. But it's not a terrible offender if it were in a minor version bump.

same person a minute later:

I think a minor version bump is somewhat reasonable still

Someone else, on Twitter:

I see the gray area but would personally put it as a patch version - error messages shouldn't be considered public, that is what error numbers or codes are for. Having said that, sorta depends on how it was documented and if there were error codes to consume.

Related: https://semver.org/#what-if-i-inadvertently-alter-the-public-api-in-a-way-that-is-not-compliant-with-the-version-number-change-ie-the-code-incorrectly-introduces-a-major-breaking-change-in-a-patch-release

[timdorr]

You can use the Netlify preview to see it in action: https://deploy-preview-3920--redux-docs.netlify.app/errors?code=5

[markerikson]

That is slick.

Well, I'm sold. Tim, thoughts?

[timdorr]

I think it looks pretty good. Any outstanding TODOs that I may have missed?

[andrewmcgivery]

MR is all good to go from my perspective!

The only note I will add is I would be a little hesitant to switch all of the redux repos to TSDX unless there is a good reason to based on the issues around error extraction not working and it being extremely difficult to debug why. I'm sure TSDX is great if you're starting from scratch and don't want to deal with rollup configuration but I would argue if you've already got a config that is working, I don't see the benefit to changing that. Just my 2 cents. :)

[markerikson]

FWIW, it sounds like we were looking at dropping TSDX for the next RTK release anyway due to needing to have multiple entry points, so that might not be an issue for long.

[markerikson]

@andrewmcgivery yeah, let's go ahead and get this into master ASAP. Anything you need from me on that end?

[andrewmcgivery]

@markerikson Don't think so. Working on this now. :)

[markerikson]

Great :)

We've got a big RTK 1.6 release we're working towards, and it would be good if there was a Redux 4.1 with the error code improvements we could include in that.

[andrewmcgivery]

@markerikson So... hit a bit of a snag but also have a workaround... but want to be really transparent about it. :)

So, I seem to be running into an issue with the preflightCheck in @rollup/plugin-babel where it hits "An unexpected situation":

Looking at the preflight code itself, I'm really stumped why this is happening at all. Based on my debugging, it seems to be having a problem when it adds the import for the formatProdErrorMessage function, however, it appears to be perfectly valid code and if I turn off the preflight check, it seems to build fine.

The preflight check code:

const inheritsHelperRe = /\/helpers\/(esm\/)?inherits/;
async function preflightCheck(ctx, babelHelpers, transformOptions) {
  const finalOptions = addBabelPlugin(transformOptions, helpersTestTransform);
  const check = (await babel.transformAsync(PREFLIGHT_INPUT, finalOptions)).code; // Babel sometimes splits ExportDefaultDeclaration into 2 statements, so we also check for ExportNamedDeclaration

  if (!/export (d|{)/.test(check)) {
    ctx.error(MODULE_ERROR);
  }

  if (inheritsHelperRe.test(check)) {
    if (babelHelpers === RUNTIME) {
      return;
    }

    ctx.error(mismatchError(RUNTIME, babelHelpers, transformOptions.filename));
  }

  if (check.includes('babelHelpers.inherits')) {
    if (babelHelpers === EXTERNAL) {
      return;
    }

    ctx.error(mismatchError(EXTERNAL, babelHelpers, transformOptions.filename));
  } // test unminifiable string content


  if (check.includes('Super expression must either be null or a function')) {
    if (babelHelpers === INLINE || babelHelpers === BUNDLED) {
      return;
    }

    if (babelHelpers === RUNTIME && !transformOptions.plugins.length) {
      ctx.error(`You must use the \`@babel/plugin-transform-runtime\` plugin when \`babelHelpers\` is "${RUNTIME}".\n`);
    }

    ctx.error(mismatchError(INLINE, babelHelpers, transformOptions.filename));
  }

  // This is where the error comes from
  ctx.error(UNEXPECTED_ERROR);
}

The code it is checking when it errors looks like this which appears to be perfectly valid:

import _formatProdErrorMessage from "src/utils/formatProdErrorMessage";

function _inherits(subClass, superClass) { if (typeof superClass !== "function" && superClass !== null) { throw new Error(_formatProdErrorMessage(16)); } subClass.prototype = Object.create(superClass && superClass.prototype, { constructor: { value: subClass, writable: true, configurable: true } }); if (superClass) _setPrototypeOf(subClass, superClass); }    

function _setPrototypeOf(o, p) { _setPrototypeOf = Object.setPrototypeOf || function _setPrototypeOf(o, p) { o.__proto__ = p; return o; }; return _setPrototypeOf(o, p); }

export default _inherits;

So, I'm THINKING this is a weird false positive. It only comes up when building ES for Browsers and UMD Production (since those are the two builds where I am doing the error minify on build). I've added skipPreflightCheck to the rollup config for those two builds and it seems to fix it. As far as I can tell from the plugin configuration (it's a bit vague), it is safe to turn this off "if you are confident about your configuration". (see: https://github.com/rollup/plugins/tree/master/packages/babel#skippreflightcheck)

I've dug about as far as I can on this one without my brain leaking out of my ears (lol). Are you comfortable with having this flag for those two builds?

[markerikson]

Whoa, that's weird. Maybe worth filing an issue over in the Rollup plugins repo?

I'd at least like to better understand what's going on here before we merge that.

[markerikson]

Any further info on what's going on with that "preflight" issue?

[Andarist]

This is, indeed, an unexpected situation and using the suppressing flag might a good fix for this.

For npm builds u are using @babel/runtime helpers so you mark them as external and for UMDs and similar u don't do that, since u want to bundle this stuff in. Some Babel helpers have errors in them - so your error extractor replaces them with codes and thus this check fails:
https://github.com/rollup/plugins/blob/c328e2cfc0d727cd964a317746b299ae429fb00c/packages/babel/src/preflightCheck.js#L63
While for regular cases this works and usually that string is a sufficiently good sign that the containing function is a Babel helper.

[markerikson]

Gotcha. So, it sounds like that means that A) this is safe to bypass, and B) this PR is sufficiently good to go.

Hmm. My only other question at this point is how to handle the fact that we're deploying the website off of master, but I'll be porting this PR over to 4.x and doing our next 4.1.0 release off there. I guess I ought to do any error message changes in both places so that they stay consistent? I mean, I should do that anyway so that there's no weird message reversions whenever we (someday) publish 5.0, but it'd be necessary to make sure that error codes from 4.1 show up on the website correctly.

[markerikson]

@andrewmcgivery one other question here. It looks like the React error codes approach allows doing some string substitutions, like: "7": "Expected %s target to be an array; got %s",. Does this PR support that? If not, how difficult would it be to add that?

[andrewmcgivery]

@markerikson When I looked at how React handles that, it would involve a fair bit of refactoring to make sure we are using template literals everywhere in the codebase and consistently. React uses invariant everywhere instead of throw new Error which... I don't know if that makes a difference but it's worth noting.

So... is it possible? Yes. But I feel it would be a fair bit of work to actually do it on this PR. Would definitely hold this up for an unknown amount of time.

[markerikson]

Awright, probably not worth it for now, then. Can always revisit that later.

I've currently got this branch checked out and am playing around with a couple other things, like reworking the "use middleware for async actions" message to correctly capture the type of the value that was dispatched.

The output of the build step here looks good and I'll probably merge this shortly, and then I'll port this over to 4.x.

[markerikson]

Y'know what, let's do this. Merging now, and will follow up with a PR that reworks a bunch of our error messages (per #4028 ). From there I'll backport all those changes to 4.x.

Thank you so much for your efforts on this PR!

[andrewmcgivery]

Thanks @markerikson!

[markerikson]

Y'know, it's sad that we didn't get to see the actual size diff in this PR - I think the "compressed size action" we currently use doesn't work right for external PRs. (Supposedly there's a "compressed size bot" we can use instead that will work better Scratch that. We apparently looked at its source and didn't like it: reduxjs/redux-toolkit#635 ).

However, we should get a similar result when I port this over into the 4.x branch shortly.

edit

Oh here we go - it does run, it just doesn't auto-add the comment for some reason.

Pasting the output from the job:

Size Change: -684 B (8%) ✅

Total Size: 8.21 kB

Filename	Size	Change
dist/redux.js	6.37 kB	-3 B (0%)
dist/redux.min.js	1.84 kB	-681 B (36%)	🎉

Looks like a solid win to me, and the changes I just made in #4055 drop another 138 bytes off that (down to 1.71k).