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

Tracks side effects of thenables #4115

Merged
merged 2 commits into from May 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/ast/nodes/AwaitExpression.ts
@@ -1,3 +1,5 @@
import { NormalizedTreeshakingOptions } from '../../rollup/types';
import { NO_ARGS } from '../CallOptions';
import { HasEffectsContext, InclusionContext } from '../ExecutionContext';
import ArrowFunctionExpression from './ArrowFunctionExpression';
import * as NodeType from './NodeType';
Expand All @@ -9,7 +11,20 @@ export default class AwaitExpression extends NodeBase {
type!: NodeType.tAwaitExpression;

hasEffects(context: HasEffectsContext): boolean {
return !context.ignore.returnAwaitYield || this.argument.hasEffects(context);
const { propertyReadSideEffects } = this.context.options
.treeshake as NormalizedTreeshakingOptions;
return (
!context.ignore.returnAwaitYield ||
this.argument.hasEffects(context) ||
this.argument.hasEffectsWhenCalledAtPath(
['then'],
{ args: NO_ARGS, thisParam: null, withNew: false },
context
) ||
(propertyReadSideEffects &&
(propertyReadSideEffects === 'always' ||
this.argument.hasEffectsWhenAccessedAtPath(['then'], context)))
);
}

include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
Expand Down
16 changes: 14 additions & 2 deletions src/ast/nodes/shared/FunctionNode.ts
@@ -1,4 +1,4 @@
import { CallOptions } from '../../CallOptions';
import { CallOptions, NO_ARGS } from '../../CallOptions';
import { BROKEN_FLOW_NONE, HasEffectsContext, InclusionContext } from '../../ExecutionContext';
import { EVENT_CALLED, NodeEvent } from '../../NodeEvents';
import FunctionScope from '../../scopes/FunctionScope';
Expand Down Expand Up @@ -56,7 +56,7 @@ export default class FunctionNode extends NodeBase {
}

getReturnExpressionWhenCalledAtPath(path: ObjectPath): ExpressionEntity {
return path.length === 0 ? this.scope.getReturnExpression() : UNKNOWN_EXPRESSION;
return !this.async && path.length === 0 ? this.scope.getReturnExpression() : UNKNOWN_EXPRESSION;
}

hasEffects(): boolean {
Expand All @@ -81,6 +81,18 @@ export default class FunctionNode extends NodeBase {
context: HasEffectsContext
): boolean {
if (path.length > 0) return true;
if (
this.async &&
this.scope
.getReturnExpression()
.hasEffectsWhenCalledAtPath(
['then'],
{ args: NO_ARGS, thisParam: null, withNew: false },
context
)
) {
return true;
}
for (const param of this.params) {
if (param.hasEffects(context)) return true;
}
Expand Down
7 changes: 4 additions & 3 deletions src/ast/nodes/shared/knownGlobals.ts
Expand Up @@ -191,10 +191,11 @@ const knownGlobals: GlobalDescription = {
Promise: {
__proto__: null,
[ValueProperties]: IMPURE,
all: PF,
all: O,
prototype: O,
race: PF,
resolve: PF
race: O,
reject: O,
resolve: O
},
propertyIsEnumerable: O,
Proxy: O,
Expand Down
5 changes: 5 additions & 0 deletions test/form/samples/async-function-effects/_config.js
@@ -0,0 +1,5 @@
const path = require('path');

module.exports = {
description: 'tracks effects when awaiting thenables'
};
74 changes: 74 additions & 0 deletions test/form/samples/async-function-effects/_expected.js
@@ -0,0 +1,74 @@
(async function () {
return {
then() {
console.log(1);
}
};
})();

(async function () {
await {
then: function () {
console.log(2);
}
};
return { then() {} };
})();

(async function () {
await {
get then() {
console.log(3);
return () => {};
}
};
return { then() {} };
})();

(async function () {
await {
get then() {
return () => console.log(4);
}
};
return { then() {} };
})();

(async function () {
await await {
then(resolve) {
resolve({
then() {
console.log(5);
}
});
}
};
return { then() {} };
})();

async function asyncIdentity(x) {
return x;
}

asyncIdentity({}); // no side effects - may be dropped

const promise = asyncIdentity(6);

promise.then(x => console.log(x));

asyncIdentity({
then(success, fail) {
success(console.log(7));
}
});

asyncIdentity({
then(resolve) {
resolve({
then() {
console.log(8);
}
});
}
});
97 changes: 97 additions & 0 deletions test/form/samples/async-function-effects/main.js
@@ -0,0 +1,97 @@
(async function () {
return {
then() {
console.log(1);
}
};
})();

// removed
(async function () {
return { then() {} };
})();

(async function () {
await {
then: function () {
console.log(2);
}
};
return { then() {} };
})();

// removed
(async function () {
await {
then: function () {}
};
return { then() {} };
})();

(async function () {
await {
get then() {
console.log(3);
return () => {};
}
};
return { then() {} };
})();

(async function () {
await {
get then() {
return () => console.log(4);
}
};
return { then() {} };
})();

// removed
(async function () {
await {
get then() {
return () => {};
}
};
return { then() {} };
})();

(async function () {
await await {
then(resolve) {
resolve({
then() {
console.log(5);
}
});
}
};
return { then() {} };
})();

async function asyncIdentity(x) {
return x;
}

asyncIdentity({}); // no side effects - may be dropped

const promise = asyncIdentity(6);

promise.then(x => console.log(x));

asyncIdentity({
then(success, fail) {
success(console.log(7));
}
});

asyncIdentity({
then(resolve) {
resolve({
then() {
console.log(8);
}
});
}
});
3 changes: 1 addition & 2 deletions test/form/samples/promises/_config.js
@@ -1,4 +1,3 @@
module.exports = {
description: 'do not remove promise creations',
options: { output: { name: 'bundle' } }
description: 'do not remove promise creations'
};
49 changes: 49 additions & 0 deletions test/form/samples/promises/_expected.js
@@ -0,0 +1,49 @@
Promise.resolve({
then() {
console.log(1);
}
});

Promise.resolve({
get then() {
return () => console.log(2);
}
});

Promise.reject('should be kept for uncaught rejections');

Promise.all([
{
then() {
console.log(3);
}
},
null
]);

Promise.all([
null,
{
get then() {
return () => console.log(4);
}
}
]);

Promise.race([
{
then() {
console.log(5);
}
},
null
]);

Promise.race([
null,
{
get then() {
return () => console.log(6);
}
}
]);
21 changes: 0 additions & 21 deletions test/form/samples/promises/_expected/amd.js

This file was deleted.

19 changes: 0 additions & 19 deletions test/form/samples/promises/_expected/cjs.js

This file was deleted.

15 changes: 0 additions & 15 deletions test/form/samples/promises/_expected/es.js

This file was deleted.

24 changes: 0 additions & 24 deletions test/form/samples/promises/_expected/iife.js

This file was deleted.