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.

Sign up for GitHub

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

Jump to bottom

Detect async arrow thenable side effects #4120

Merged
merged 3 commits into from Jun 3, 2021
Merged

Detect async arrow thenable side effects #4120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
10148c6
Respect propertyReadSideEffects in spread elements
lukastaegert Jun 2, 2021
77dbd78
Track async arrow return effects
lukastaegert Jun 2, 2021
09bb382
Merge branch 'master' into gh-4116-async-arrow-thenables
lukastaegert Jun 3, 2021
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
23 changes: 21 additions & 2 deletions src/ast/nodes/ArrowFunctionExpression.ts
View file
Open in desktop
@@ -1,4 +1,5 @@
import { CallOptions } from '../CallOptions';
import { NormalizedTreeshakingOptions } from '../../rollup/types';
import { CallOptions, NO_ARGS } from '../CallOptions';
import { BROKEN_FLOW_NONE, HasEffectsContext, InclusionContext } from '../ExecutionContext';
import ReturnValueScope from '../scopes/ReturnValueScope';
import Scope from '../scopes/Scope';
Expand All @@ -13,6 +14,7 @@ import { ExpressionNode, GenericEsTreeNode, IncludeChildren, NodeBase } from './
import { PatternNode } from './shared/Pattern';

export default class ArrowFunctionExpression extends NodeBase {
async!: boolean;
body!: BlockStatement | ExpressionNode;
params!: PatternNode[];
preventChildBlockScope!: true;
Expand All @@ -35,7 +37,7 @@ export default class ArrowFunctionExpression extends NodeBase {
deoptimizeThisOnEventAtPath(): void {}

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 @@ -56,6 +58,23 @@ export default class ArrowFunctionExpression extends NodeBase {
context: HasEffectsContext
): boolean {
if (path.length > 0) return true;
if (this.async) {
const { propertyReadSideEffects } = this.context.options
.treeshake as NormalizedTreeshakingOptions;
const returnExpression = this.scope.getReturnExpression();
if (
returnExpression.hasEffectsWhenCalledAtPath(
['then'],
{ args: NO_ARGS, thisParam: null, withNew: false },
context
) ||
(propertyReadSideEffects &&
(propertyReadSideEffects === 'always' ||
returnExpression.hasEffectsWhenAccessedAtPath(['then'], context)))
) {
return true;
}
}
for (const param of this.params) {
if (param.hasEffects(context)) return true;
}
Expand Down
22 changes: 14 additions & 8 deletions src/ast/nodes/shared/FunctionNode.ts
View file
Open in desktop
@@ -1,3 +1,4 @@
import { NormalizedTreeshakingOptions } from '../../../rollup/types';
import { CallOptions, NO_ARGS } from '../../CallOptions';
import { BROKEN_FLOW_NONE, HasEffectsContext, InclusionContext } from '../../ExecutionContext';
import { EVENT_CALLED, NodeEvent } from '../../NodeEvents';
Expand Down Expand Up @@ -81,17 +82,22 @@ export default class FunctionNode extends NodeBase {
context: HasEffectsContext
): boolean {
if (path.length > 0) return true;
if (
this.async &&
this.scope
.getReturnExpression()
.hasEffectsWhenCalledAtPath(
if (this.async) {
const { propertyReadSideEffects } = this.context.options
.treeshake as NormalizedTreeshakingOptions;
const returnExpression = this.scope.getReturnExpression();
if (
returnExpression.hasEffectsWhenCalledAtPath(
['then'],
{ args: NO_ARGS, thisParam: null, withNew: false },
context
)
) {
return true;
) ||
(propertyReadSideEffects &&
(propertyReadSideEffects === 'always' ||
returnExpression.hasEffectsWhenAccessedAtPath(['then'], context)))
) {
return true;
}
}
for (const param of this.params) {
if (param.hasEffects(context)) return true;
Expand Down
2 changes: 0 additions & 2 deletions test/form/samples/async-function-effects/_config.js
View file
Open in desktop
@@ -1,5 +1,3 @@
const path = require('path');

module.exports = {
description: 'tracks effects when awaiting thenables'
};
50 changes: 43 additions & 7 deletions test/form/samples/async-function-effects/_expected.js
View file
Open in desktop
Expand Up @@ -6,10 +6,46 @@
};
})();

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

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

(async () => ({
then() {
console.log(4);
}
}))();

(async () => ({
get then() {
console.log(5);
return () => {};
}
}))();

(async () => ({
get then() {
return () => console.log(6);
}
}))();

(async function () {
await {
then: function () {
console.log(2);
console.log(7);
}
};
return { then() {} };
Expand All @@ -18,7 +54,7 @@
(async function () {
await {
get then() {
console.log(3);
console.log(8);
return () => {};
}
};
Expand All @@ -28,7 +64,7 @@
(async function () {
await {
get then() {
return () => console.log(4);
return () => console.log(9);
}
};
return { then() {} };
Expand All @@ -39,7 +75,7 @@
then(resolve) {
resolve({
then() {
console.log(5);
console.log(10);
}
});
}
Expand All @@ -53,21 +89,21 @@ async function asyncIdentity(x) {

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

const promise = asyncIdentity(6);
const promise = asyncIdentity(11);

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

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

asyncIdentity({
then(resolve) {
resolve({
then() {
console.log(8);
console.log(13);
}
});
}
Expand Down
71 changes: 64 additions & 7 deletions test/form/samples/async-function-effects/main.js
View file
Open in desktop
Expand Up @@ -11,10 +11,67 @@
return { then() {} };
})();

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

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

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

(async () => ({
then() {
console.log(4);
}
}))();

// removed
(async () => ({
then() {}
}))();

(async () => ({
get then() {
console.log(5);
return () => {};
}
}))();

(async () => ({
get then() {
return () => console.log(6);
}
}))();

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

(async function () {
await {
then: function () {
console.log(2);
console.log(7);
}
};
return { then() {} };
Expand All @@ -31,7 +88,7 @@
(async function () {
await {
get then() {
console.log(3);
console.log(8);
return () => {};
}
};
Expand All @@ -41,7 +98,7 @@
(async function () {
await {
get then() {
return () => console.log(4);
return () => console.log(9);
}
};
return { then() {} };
Expand All @@ -62,7 +119,7 @@
then(resolve) {
resolve({
then() {
console.log(5);
console.log(10);
}
});
}
Expand All @@ -76,21 +133,21 @@ async function asyncIdentity(x) {

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

const promise = asyncIdentity(6);
const promise = asyncIdentity(11);

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

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

asyncIdentity({
then(resolve) {
resolve({
then() {
console.log(8);
console.log(13);
}
});
}
Expand Down
16 changes: 16 additions & 0 deletions test/form/samples/ignore-property-access-side-effects/main.js
View file
Open in desktop
Expand Up @@ -24,6 +24,22 @@ const foo5 = {
};

const foo6 = (async function () {
return {
get then() {
console.log('effect');
return () => {};
}
};
})();

const foo7 = (async () => ({
get then() {
console.log('effect');
return () => {};
}
}))();

const foo8 = (async function () {
await {
get then() {
console.log('effect');
Expand Down
20 changes: 18 additions & 2 deletions test/form/samples/keep-property-access-side-effects/_expected.js
View file
Open in desktop
@@ -1,6 +1,6 @@
const getter = {
get foo () {
console.log( 'effect' );
get foo() {
console.log('effect');
}
};
getter.foo;
Expand All @@ -23,6 +23,22 @@ globalThis.unknown.unknownProperty;
}
});

((async function () {
return {
get then() {
console.log('effect');
return () => {};
}
};
}))();

(async () => ({
get then() {
console.log('effect');
return () => {};
}
}))();

((async function () {
await {
get then() {
Expand Down
20 changes: 18 additions & 2 deletions test/form/samples/keep-property-access-side-effects/main.js
View file
Open in desktop
@@ -1,6 +1,6 @@
const getter = {
get foo () {
console.log( 'effect' );
get foo() {
console.log('effect');
}
};
const foo1 = getter.foo;
Expand All @@ -24,6 +24,22 @@ const foo5 = {
};

const foo6 = (async function () {
return {
get then() {
console.log('effect');
return () => {};
}
};
})();

const foo7 = (async () => ({
get then() {
console.log('effect');
return () => {};
}
}))();

const foo8 = (async function () {
await {
get then() {
console.log('effect');
Expand Down