Handling expressions of non-literals

[guybedford]

Seeing #2214 made me wonder about the following case.

What would be involved to extend the current literal value logic to deal with cases like:

const obj = {
	flag: false
};

function Reassign(x) {
	x.flag = true;
}

new Reassign(obj);

let hasBeenReassigned = false;

if (obj) {
	hasBeenReassigned = true;
}

assert.ok(hasBeenReassigned);

where the object itself being to known to be an object implies its truthiness in logical expressions.

[lukastaegert]

I think this would be only moderately difficult and the kind of transformation where TypeScript might really help us. Going with the current logic, we could add new pseudo values similar to UNKNOWN_VALUE. Those values could be something like UNKNOWN_TRUTHY_VALUE or UNKNOWN_OBJECT_VALUE.

Then we would just go through all usages of getLiteralValue and make sure something sensible happens if one of the new values is returned. We also might want to make sure that it is easy to detect the UNKNOWN_* values as I think there will be places (binary operator?) where we often want to return UNKNOWN_VALUE when one value is of the unknown kind (or maybe not—JavaScript has some funky auto type casts...)

While we are at it I would also suggest to extend the logic to handle explicit undefined as well as implicit undefined for missing object properties.

[guybedford]

I'm actually looking into this a little at the moment, and I wonder if this should be implemented as an EntityExpression or as a type of value?

[guybedford]

The more I look at this the more I think handling cases like ('asdf' + 'asdf').substr(5) requires using the entity system over the literal value system? Let me know your thoughts.

[lukastaegert]

I have thought a little about this and to me, this seems to be an issue very specific to binary expressions as they are on of the few expressions generating "new primitives" (the other being update expressions). Other kinds of expressions (e.g. binary expressions) already handle these cases fine.

I would go for a localized solution in BinaryExpression:

• hasEffectsWhenCalledAtPath should use its own getLiteralValueAtPath function to retrieve its own literal value and, if it is not "unknown", use something like "typeof" to determine which type of literal it is. Then it could behave like the given literal type.
• The same approach could be used for getReturnExpressionWhenCalledAtPath and other methods.
• Note that as we are not dynamically binding yet and it will probably happen in a different way anyway, there should be no harm in determining the literal value once (after the bind phase, e.g. on first request) and caching the result.

If you want to have a shot at this, please go ahead! I hope to finally get your pending PRs reviewed tomorrow... (all of which seem to be of the new feature category so they should probably go into the 1.0.0 anyway?)

[guybedford]

I'm also tempted to try and inline intermediate computed values - eg having 'asdf' + 'asdf' automatically rewrite as 'asdfasdf', since we are doing this work anyway.

As for 1.0, I'm very much a proponent of a "symbolic" 1.0, that is having all the features already present, and then simply removing the deprecation paths.

[lukastaegert]

I'm also tempted to try and inline intermediate computed values

Inlining would also make sense but with the current architecture, I would propose to keep this separate from the tree-shaking and do it in the rendering phase as these are two separate issues (you probably want to inline 'x' + 'y' but not someVariable + 'y' if someVariable is used in many places and a possibly very long string).

As for 1.0, I'm very much a proponent of a "symbolic" 1.0, that is having all the features already present, and then simply removing the deprecation paths.

With regard to the current issue, what deprecation paths are you talking about?

[guybedford]

In this case, switching --experimentalCodeSplitting on by default.

Other deprecations include deprecating the transformBundle hook, onwrite, entry etc there are a few things about.

[lukastaegert]

Ah, you are referring to my last comment. I would hope not to release a 0.61.0 before the 1.0.0 but regarding my comment on Twitter DM, I would start releasing pre-release versions.

[guybedford]

Made some good progress here actually. The hardest thing about this work is handling the feature detection cases like typeof Object.create === 'undefined'....

[lukastaegert]

Why would you even want to implement "typeof" for a non-literal when there are more things that can be broken than fixed by that

[lukastaegert]

Please disregard my previous comment. With regard to feature detection, please see my comments at #2273. As such, I hope you do not try to solve everything in one PR (i.e. global handling, feature detection) as this will also make reviewing much harder and tree-shaking logic is something that can easily break code for others without us noticing so it requires a lot of careful reviewing (and testing).

If you think there are critical exceptions to be made, maybe leave certain situations completely unoptimized for now and we can talk about that and improve in separate PRs?

[guybedford]

I've put this work on pause for now... was a very interesting process and learning experience working on it and got things like value inlining going even. Will aim to pick this up again when I next have time (a few weeks yet) and separate it from feature detection work.

[shellscape]

Hey folks. This is a saved-form message, but rest assured we mean every word. The Rollup team is attempting to clean up the Issues backlog in the hopes that the active and still-needed, still-relevant issues bubble up to the surface. With that, we're closing issues that have been open for an eon or two, and have gone stale like pirate hard-tack without activity.

We really appreciate the folks have taken the time to open and comment on this issue. Please don't confuse this closure with us not caring or dismissing your issue, feature request, discussion, or report. The issue will still be here, just in a closed state. If the issue pertains to a bug, please re-test for the bug on the latest version of Rollup and if present, please tag @shellscape and request a re-open, and we'll be happy to oblige.