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

Save 8kb by replaceing error messages with error codes #653

Closed
5 tasks done
marvinhagemeister opened this issue Nov 13, 2016 · 9 comments
Closed
5 tasks done

Save 8kb by replaceing error messages with error codes #653

marvinhagemeister opened this issue Nov 13, 2016 · 9 comments

Comments

@marvinhagemeister
Copy link

  • What problem would it solve for you?
  • Do you think others will benefit from this change as well?
  • Are you willing to (attempt) a PR yourself?
  • Are you willing to provide unit tests?
  • Are you willing to update the docs?

I'm currently looking into ways to make the bundle size smaller and noticed that we'd save a lot by swapping out the error messages with error codes similar to what the react guys did. This would save about ~8KB (53 vs 45 kb) for the non-gzipped minified bundle.

We could print something like this:

MobX Error #109; visit https://example.com/docs/error/109 for the full message or
use the non-minified dev environment for full errors and additional helpful warnings.
@marvinhagemeister
Copy link
Author

Plan of action

After a quick research, the best way forward seems to be a custom babel plugin. That way we can easily generate different bundles by changing either NODE_ENV or BABEL_ENV environment variables.

There is no way around parsing the code anyway, and babel's ast-parser is the most advanced and best documented at the time of this proposal. Typescript itself doesn't have an api for plugins (yet?).

Behind the scenes react parses the source code and searches the ast tree for a specific function call expression and replaces the call with a different error function. In our case we have to simply look for a CallExpression named invariant. The error message itself is extracted into a simple lookup table:

{
  "My error message": "0",
  "Another error message": "1"
  // ...
}

We replace the message parameters (like component name) with simple placeholders, so that we can effectively prevent duplicate messages.

Requirements

The proposed approach has a few hard requirements, that should be kept in mind:

  • The url must be stable and never change (bad for users of old versions of mobx)
  • The error code itself must obviously consecutively numbered or the above would break

I'll post an update once I'll have some code ready to comment on.

@mweststrate
Copy link
Member

For urls: best create a github issue, there is one that follows this mechanism already :) https://github.com/mobxjs/mobx/issues?q=label%3Abuilt-in-error-explanation+is%3Aclosed

Personally I think using the same approach as React will be the most convenient, as it makes it the responsibility of the uglifier to strip out the additional code. A simple table with issue numbers, and two different implementations should suffice here I think

@andykog
Copy link
Member

andykog commented Nov 16, 2016

@mweststrate, I assume, we won't create issue for every error key, I'd better create simple page on gh-pages, thats loads errors.json from github and add link to search for issues there.

@marvinhagemeister, you know there is typescript ast parser, right? May be using it like a simple browserify plugin (example) would work?

@marvinhagemeister
Copy link
Author

marvinhagemeister commented Nov 16, 2016

@andykog Awesome, thanks for sharing, I didn't know about that one!

I'm not sold on linking to github issues. A page on gh-pages seems like a less confusing approach imo.

@benjamingr
Copy link
Member

We use goo.gl links at bluebird that direct to pages with a lot of info - it was useful in Angular, I recommend those.

@mweststrate
Copy link
Member

gh-pages section sounds fine as well :) Volunteers are welcome!

@dreisel
Copy link

dreisel commented Jan 20, 2017

I've opened a PR as part one of the issue. I've transferred String messages to a separate file.
Many strings though are used with string literals and objects, so may by good concerning a string formatting library and to export a function that can compute the requested output.
#GoodnessSquad

#783

@mweststrate
Copy link
Member

Closed since there is a follow up issue #813

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

No branches or pull requests

5 participants