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

Longer term: make uglifyjs work out-of-the-box on browserified, custom userland code that builds RttcRefPlaceholders (currently you have to provide uglify w/ an extra config option) #3

Open
mikermcneil opened this issue Sep 30, 2016 · 1 comment

Comments

@mikermcneil
Copy link
Member

mikermcneil commented Sep 30, 2016

Currently, when browserifying and then also uglifying custom userland code that builds RttcRefPlaceholder instances, you will run into issues-- at least with default uglifyjs settings.

Huh? Why?

This is because uglify strips function names, effectively ripping off the 'RttcRefPlaceholder' moniker. This makes code that does constructor.name === 'RttcRefPlaceholder' sniffing fail.

This type of sniffing is built into several RTTC methods. And unless you are doing some very specific stuff in your userland code, it probably doesn't affect you.... but just in case it does, or if you're curious:

What is RttcRefPlaceholder?

rttc uses RttcRefPlaceholder instances as a special way to pass in a "platonic value" which only validates as a ref.

Every other rttc display type has at least one value that validates only as that type, or as a more generic type . For example, any function works as a sample value that will only validate as a lamda (->) or ref (===). And the null literal (null) works as a sample value that will only validate as a json (*) or ref (===). But what about ref? That's where RttcRefPlaceholder comes in.

Here's an example of what some relevant userland code might look like:

// Build the abstract, platonic ideal of "ref" 
var PlatonicRefConstructor = function RttcRefPlaceholder() {};
var thePlatonicRef = new PlatonicRefConstructor();

assert(thePlatonicRef.constructor.name === 'RttcRefPlaceholder');
// (But if you browserify and then uglify this, `thePlatonicRef.constructor.name` will be different.)

Would I ever need to construct an RttcRefPlaceholder?

The only time you need to construct instances of RttcRefPlaceholder is if you building tooling on top of rttc and/or the machine runner: something like a test runner, IDE plugin, transformer, or anything doing static analysis. If you are not doing stuff like that in your custom userland code, or if you are, but you're not browserifying and then also uglifying it, then this issue does not affect you.

Recommended solution

Caveats aside, if this issue applies to you, here's how to solve it.

The simplest, naive workaround is to set mangle: false in uglify's settings. But this can substantially increase the final size of your minified bundle (in our test case, the difference was 2,141,105 vs 1,636,170 bytes. So ~500kb or 30% larger.)

A better option is to set mangle: { except: ['RttcRefPlaceholder'] }. This specifically prevents the mangling of functions with this name-- and fixes the problem with a negligible impact on the file size of the minified bundle generated by uglify (in our example above, it adds 68 bytes ~~a difference of less than 50 unicode characters in the output).

Longer term strategy

The most likely way to solve this longer term is to use duck-typing for RttcRefPlaceholder checks, instead of relying on constructor.name === 'RttcRefPlaceholder. To do that, we can check for a "telltale", an extra, private, non-enumerable property on the function (similar to what we do w/ switchback, in Waterline, and elsewhere). However, this may come with its own can of worms that will need to be carefully tested (think enumerability and visibility, even usability stuff like console.log and toString() output).

@mikermcneil
Copy link
Member Author

This "mangle except" opt is now included out of the box in the config/uglify.js task for newly-generated Sails apps as of Sails v1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant