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

Ensure the initializer of a destructuring declaration is always included if the id is included #4663

Merged
merged 2 commits into from Oct 12, 2022
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
13 changes: 12 additions & 1 deletion src/ast/nodes/VariableDeclaration.ts
Expand Up @@ -15,8 +15,10 @@ import {
import type { InclusionContext } from '../ExecutionContext';
import { EMPTY_PATH } from '../utils/PathTracker';
import type Variable from '../variables/Variable';
import ArrayPattern from './ArrayPattern';
import Identifier, { type IdentifierWithVariable } from './Identifier';
import * as NodeType from './NodeType';
import ObjectPattern from './ObjectPattern';
import type VariableDeclarator from './VariableDeclarator';
import type { InclusionOptions } from './shared/Expression';
import { type IncludeChildren, NodeBase } from './shared/Node';
Expand Down Expand Up @@ -62,8 +64,17 @@ export default class VariableDeclaration extends NodeBase {
for (const declarator of this.declarations) {
if (includeChildrenRecursively || declarator.shouldBeIncluded(context))
declarator.include(context, includeChildrenRecursively);
const { id, init } = declarator;
if (asSingleStatement) {
declarator.id.include(context, includeChildrenRecursively);
id.include(context, includeChildrenRecursively);
}
if (
init &&
id.included &&
!init.included &&
(id instanceof ObjectPattern || id instanceof ArrayPattern)
) {
init.include(context, includeChildrenRecursively);
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions test/function/samples/for-loop-parameter/_config.js
@@ -0,0 +1,9 @@
const assert = require('assert');

module.exports = {
description: 'includes for-loop parameters',
exports({ checkObject, checkArray }) {
assert.strictEqual(checkObject({ foo: 1 }), 1, 'object');
assert.strictEqual(checkArray([2]), 2, 'array');
}
};
19 changes: 19 additions & 0 deletions test/function/samples/for-loop-parameter/main.js
@@ -0,0 +1,19 @@
export function checkObject(p) {
return getFromObjectInLoop(p);
}

export function checkArray(p) {
return getFromArrayInLoop(p);
}

function getFromObjectInLoop(path) {
for (let { foo } = path;;) {
return foo;
}
}

function getFromArrayInLoop(path) {
for (let [ foo ] = path;;) {
return foo;
}
}
9 changes: 5 additions & 4 deletions test/watch/index.js
Expand Up @@ -61,10 +61,11 @@ describe('rollup.watch', () => {
}
});
} else {
Promise.resolve()
.then(() => wait(timeout)) // gah, this appears to be necessary to fix random errors
.then(() => next(event))
.then(go)
wait(timeout) // gah, this appears to be necessary to fix random errors
.then(() => {
next(event);
go();
})
.catch(error => {
watcher.close();
reject(error);
Expand Down