Proposal for marking functions as pure

[nathancahill]

UPDATED PROPOSAL

UglifyJS2, as of 2.8.0, now supports marking pure with this comment:

/*#__PURE__*/

I propose that Rollup adopt similar behavior to help with tree-shaking.

PREVIOUS PROPOSAL (before UglifyJS release)

I'm proposing adding eslint-style comment annotations to give users more control over tree-shaking:

tree-shaking: pure "Affected expressions will not mark their nodes for inclusion"
tree-shaking: auto "Current behavior"
tree-shaking: side-effects "Affected expressions mark their nodes for inclusion"

Overriding expression marks on a single line:

Greeting.propTypes = {  // rollup-tree-shaking pure
...
}

Overriding expression marks in a block, reverting to auto at the end of the block:

/* rollup-tree-shaking pure */
Greeting.propTypes = {
...
}

Greeting.defaultProps = {
...
};
/* rollup-tree-shaking auto */

Overriding marks for all expressions in a file:

/* rollup-tree-shaking pure */

Marking a line as having side-effects. This expression is marked for inclusion:

documentElement.style.color = 'red';  // rollup-tree-shaking side-effects

Pros

• The mutable nature of JS makes it impossible to statically analyze common statements, so a lot of potentially pure statements are left on the table when tree-shaking. These annotations allow users to say "I'm not writing maniacal JS" and get drastically smaller bundle sizes.

Cons

• Build tools that alter code before passing to rollup could shift comments to the wrong line, and apply annotations to the wrong statement(s).
• Adding program changing logic in comments. There's different camps of thought on this, but generally, comments aren't meant to alter code.
• Build tools that strip comments could remove annotations if applied in the wrong order.
• Rollup specific logic. Might make it harder to interop with other build tools.
• Could become a crutch that prevents rollup tree-shaking bugs from being fixed. If rollup is doing the wrong thing, adding the annotation would allow users to patch it and move on.
• If users don't understand the risk of overriding rollup's side-effect detection, it could cause unexpected side-effects. Maybe the pure annotation should be unsafe-pure to make the risk clear.

[Pauan]

Related:
mishoo/UglifyJS#1448
mishoo/UglifyJS#1261 (comment)
microsoft/TypeScript#13721

[nathancahill]

Well, those aren't reassuring. A lot of discussion/bike-shedding and no merged PRs. Maybe Rollup could lead the effort here since it's focus is specifically on tree-shaking?

[Pauan]

@nathancahill Well, it isn't as big of a deal in Rollup because Rollup's dead-code elimination is already really good. Notice that it was able to completely remove the unused class and propTypes annotation.

I like the idea of a comment hint, but I personally think it would be better to wait for it to be standardized in UglifyJS before adding it to Rollup, so that way both projects can share the same comment annotation.

Even if it was added to Rollup first, I don't think that would encourage UglifyJS to add it.

But I'm not a part of the Rollup team, maybe they feel differently from me.

[nathancahill]

Looks like Uglify has stabilized on using #__PURE__, as in var b = /*#__PURE__*/bar(); in the upcoming 2.8.0 release. This is on a statement-by-statement basis, which is different from my eslint-style proposal, but I consider it a good place to start. @Rich-Harris, I guess this question goes to you, is this something you want to see in Rollup?

[curran]

This would be useful for D3.js. Related discussion there: d3/d3#3076

[ghost]

Please keep Tree Skaking automatic and help keep codebases clean. Thank you.

[nathancahill]

@curran Your comment in the d3 thread nailed it, I think it's a matter of adding the check to the hasEffect function in Rollup. My toy implementation also used isUsedByBundle for marking imports as pure, but hasEffect would probably be cleaner.

[conartist6]

I'm attempting to write the diff for supporting uglify-style comments. Tracking my progress in #2423.