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

Correctly associate pure annotations and remove invalid ones #4095

Merged
merged 3 commits into from May 26, 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
4 changes: 2 additions & 2 deletions src/Graph.ts
Expand Up @@ -17,7 +17,7 @@ import { PluginDriver } from './utils/PluginDriver';
import { BuildPhase } from './utils/buildPhase';
import { errImplicitDependantIsNotIncluded, error } from './utils/error';
import { analyseModuleExecution } from './utils/executionOrder';
import { markPureCallExpressions } from './utils/pureComments';
import { addAnnotations } from './utils/pureComments';
import relativeId from './utils/relativeId';
import { timeEnd, timeStart } from './utils/timers';
import { markModuleAndImpureDependenciesAsExecuted } from './utils/traverseStaticDependencies';
Expand Down Expand Up @@ -137,7 +137,7 @@ export default class Graph {

options.onComment = onCommentOrig;

markPureCallExpressions(comments, ast, code);
addAnnotations(comments, ast, code);

return ast;
}
Expand Down
40 changes: 1 addition & 39 deletions src/Module.ts
Expand Up @@ -19,7 +19,7 @@ import Program from './ast/nodes/Program';
import TemplateLiteral from './ast/nodes/TemplateLiteral';
import VariableDeclaration from './ast/nodes/VariableDeclaration';
import { nodeConstructors } from './ast/nodes/index';
import { ExpressionNode, GenericEsTreeNode, NodeBase } from './ast/nodes/shared/Node';
import { ExpressionNode, NodeBase } from './ast/nodes/shared/Node';
import ModuleScope from './ast/scopes/ModuleScope';
import { PathTracker, UNKNOWN_PATH } from './ast/utils/PathTracker';
import ExportDefaultVariable from './ast/variables/ExportDefaultVariable';
Expand Down Expand Up @@ -62,7 +62,6 @@ import { makeLegal } from './utils/identifierHelpers';
import { basename, extname } from './utils/path';
import relativeId from './utils/relativeId';
import { RenderOptions } from './utils/renderHelpers';
import { SOURCEMAPPING_URL_COMMENT_RE } from './utils/sourceMappingURL';
import { timeEnd, timeStart } from './utils/timers';
import { markModuleAndImpureDependenciesAsExecuted } from './utils/traverseStaticDependencies';
import { MISSING_EXPORT_SHIM_VARIABLE } from './utils/variableNames';
Expand Down Expand Up @@ -121,34 +120,6 @@ const MISSING_EXPORT_SHIM_DESCRIPTION: ExportDescription = {
localName: MISSING_EXPORT_SHIM_VARIABLE
};

function findSourceMappingURLComments(ast: acorn.Node, code: string): [number, number][] {
const ret: [number, number][] = [];

const addCommentsPos = (start: number, end: number): void => {
if (start == end) {
return;
}

let sourcemappingUrlMatch;
const interStatmentCode = code.slice(start, end);
while ((sourcemappingUrlMatch = SOURCEMAPPING_URL_COMMENT_RE.exec(interStatmentCode))) {
ret.push([
start + sourcemappingUrlMatch.index,
start + SOURCEMAPPING_URL_COMMENT_RE.lastIndex
]);
}
};

let prevStmtEnd = 0;
for (const stmt of (ast as GenericEsTreeNode).body) {
addCommentsPos(prevStmtEnd, stmt.start);
prevStmtEnd = stmt.end;
}
addCommentsPos(prevStmtEnd, code.length);

return ret;
}

function getVariableForExportNameRecursive(
target: Module | ExternalModule,
name: string,
Expand Down Expand Up @@ -251,7 +222,6 @@ export default class Module {
usesTopLevelAwait = false;

private allExportNames: Set<string> | null = null;
private alwaysRemovedCode!: [number, number][];
private astContext!: AstContext;
private readonly context: string;
private customTransformCache!: boolean;
Expand Down Expand Up @@ -688,7 +658,6 @@ export default class Module {
}

setSource({
alwaysRemovedCode,
ast,
code,
customTransformCache,
Expand All @@ -700,7 +669,6 @@ export default class Module {
transformFiles,
...moduleOptions
}: TransformModuleJSON & {
alwaysRemovedCode?: [number, number][];
transformFiles?: EmittedFile[] | undefined;
}): void {
this.info.code = code;
Expand All @@ -716,11 +684,9 @@ export default class Module {

timeStart('generate ast', 3);

this.alwaysRemovedCode = alwaysRemovedCode || [];
if (!ast) {
ast = this.tryParse();
}
this.alwaysRemovedCode.push(...findSourceMappingURLComments(ast, this.info.code));

timeEnd('generate ast', 3);

Expand All @@ -734,9 +700,6 @@ export default class Module {
filename: (this.excludeFromSourcemap ? null : fileName)!, // don't include plugin helpers in sourcemap
indentExclusionRanges: []
});
for (const [start, end] of this.alwaysRemovedCode) {
this.magicString.remove(start, end);
}

timeStart('analyse ast', 3);

Expand Down Expand Up @@ -778,7 +741,6 @@ export default class Module {

toJSON(): ModuleJSON {
return {
alwaysRemovedCode: this.alwaysRemovedCode,
ast: this.ast!.esTreeNode,
code: this.info.code!,
customTransformCache: this.customTransformCache,
Expand Down
2 changes: 1 addition & 1 deletion src/ast/keys.ts
Expand Up @@ -9,7 +9,7 @@ export const keys: {

export function getAndCreateKeys(esTreeNode: GenericEsTreeNode): string[] {
keys[esTreeNode.type] = Object.keys(esTreeNode).filter(
key => typeof esTreeNode[key] === 'object' && key !== '_rollupAnnotations'
key => typeof esTreeNode[key] === 'object' && key.charCodeAt(0) !== 95 /* _ */
);
return keys[esTreeNode.type];
}
10 changes: 2 additions & 8 deletions src/ast/nodes/CallExpression.ts
Expand Up @@ -28,13 +28,7 @@ import {
UNKNOWN_EXPRESSION,
UnknownValue
} from './shared/Expression';
import {
Annotation,
ExpressionNode,
INCLUDE_PARAMETERS,
IncludeChildren,
NodeBase
} from './shared/Node';
import { ExpressionNode, INCLUDE_PARAMETERS, IncludeChildren, NodeBase } from './shared/Node';

export default class CallExpression extends NodeBase implements DeoptimizableEntity {
arguments!: (ExpressionNode | SpreadElement)[];
Expand Down Expand Up @@ -188,7 +182,7 @@ export default class CallExpression extends NodeBase implements DeoptimizableEnt
}
if (
(this.context.options.treeshake as NormalizedTreeshakingOptions).annotations &&
this.annotations?.some((a: Annotation) => a.pure)
this.annotations
)
return false;
return (
Expand Down
2 changes: 0 additions & 2 deletions src/ast/nodes/IfStatement.ts
@@ -1,6 +1,5 @@
import MagicString from 'magic-string';
import { RenderOptions } from '../../utils/renderHelpers';
import { removeAnnotations } from '../../utils/treeshakeNode';
import { DeoptimizableEntity } from '../DeoptimizableEntity';
import { BROKEN_FLOW_NONE, HasEffectsContext, InclusionContext } from '../ExecutionContext';
import TrackingScope from '../scopes/TrackingScope';
Expand Down Expand Up @@ -89,7 +88,6 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
if (includesIfElse) {
this.test.render(code, options);
} else {
removeAnnotations(this, code);
code.remove(this.start, this.consequent.start);
}
if (this.consequent.included && (noTreeshake || testValue === UnknownValue || testValue)) {
Expand Down
4 changes: 2 additions & 2 deletions src/ast/nodes/NewExpression.ts
Expand Up @@ -3,7 +3,7 @@ import { CallOptions } from '../CallOptions';
import { HasEffectsContext } from '../ExecutionContext';
import { EMPTY_PATH, ObjectPath, UNKNOWN_PATH } from '../utils/PathTracker';
import * as NodeType from './NodeType';
import { Annotation, ExpressionNode, NodeBase } from './shared/Node';
import { ExpressionNode, NodeBase } from './shared/Node';

export default class NewExpression extends NodeBase {
arguments!: ExpressionNode[];
Expand All @@ -19,7 +19,7 @@ export default class NewExpression extends NodeBase {
}
if (
(this.context.options.treeshake as NormalizedTreeshakingOptions).annotations &&
this.annotations?.some((a: Annotation) => a.pure)
this.annotations
)
return false;
return (
Expand Down
18 changes: 10 additions & 8 deletions src/ast/nodes/shared/Node.ts
Expand Up @@ -2,6 +2,7 @@ import * as acorn from 'acorn';
import { locate, Location } from 'locate-character';
import MagicString from 'magic-string';
import { AstContext } from '../../../Module';
import { ANNOTATION_KEY, INVALID_COMMENT_KEY } from '../../../utils/pureComments';
import { NodeRenderOptions, RenderOptions } from '../../../utils/renderHelpers';
import { Entity } from '../../Entity';
import {
Expand All @@ -21,13 +22,9 @@ export interface GenericEsTreeNode extends acorn.Node {

export const INCLUDE_PARAMETERS = 'variables' as const;
export type IncludeChildren = boolean | typeof INCLUDE_PARAMETERS;
export interface Annotation {
comment?: acorn.Comment;
pure?: boolean;
}

export interface Node extends Entity {
annotations?: Annotation[];
annotations?: acorn.Comment[];
context: AstContext;
end: number;
esTreeNode: GenericEsTreeNode;
Expand Down Expand Up @@ -87,7 +84,7 @@ export type StatementNode = Node;
export interface ExpressionNode extends ExpressionEntity, Node {}

export class NodeBase extends ExpressionEntity implements ExpressionNode {
annotations?: Annotation[];
annotations?: acorn.Comment[];
context: AstContext;
end!: number;
esTreeNode: acorn.Node;
Expand Down Expand Up @@ -201,8 +198,13 @@ export class NodeBase extends ExpressionEntity implements ExpressionNode {
for (const [key, value] of Object.entries(esTreeNode)) {
// That way, we can override this function to add custom initialisation and then call super.parseNode
if (this.hasOwnProperty(key)) continue;
if (key === '_rollupAnnotations') {
this.annotations = value;
if (key.charCodeAt(0) === 95 /* _ */) {
if (key === ANNOTATION_KEY) {
this.annotations = value;
} else if (key === INVALID_COMMENT_KEY) {
for (const { start, end } of value as acorn.Comment[])
this.context.magicString.remove(start, end);
}
} else if (typeof value !== 'object' || value === null) {
(this as GenericEsTreeNode)[key] = value;
} else if (Array.isArray(value)) {
Expand Down
1 change: 0 additions & 1 deletion src/rollup/types.d.ts
Expand Up @@ -114,7 +114,6 @@ export interface TransformModuleJSON extends Partial<PartialNull<ModuleOptions>>
}

export interface ModuleJSON extends TransformModuleJSON {
alwaysRemovedCode: [number, number][];
ast: AcornNode;
dependencies: string[];
id: string;
Expand Down