Skip to content

Commit

Permalink
feat: Remove building out of scope AST Nodes from resolveToValue
Browse files Browse the repository at this point in the history
BREAKING CHANGE: `resolveToValue` will not create a `MemberExpression` for targets ending in destructuring. It will now simply resolve to the `Identifier` inside the destructuring. Use new helper `isDestructuringAssignment` to further check this identifier.
  • Loading branch information
danez committed Mar 19, 2022
1 parent 4a53def commit 5bcf56c
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 42 deletions.
36 changes: 36 additions & 0 deletions src/utils/__tests__/isDestructuringAssignment-test.ts
@@ -0,0 +1,36 @@
import { parse } from '../../../tests/utils';
import isDestructuringAssignment from '../isDestructuringAssignment';

describe('isDestructuringAssignment', () => {
it('detects destructuring', () => {
const def = parse(`
var { Component } = require('react');
`).get('body', 0, 'declarations', 0, 'id', 'properties', 0, 'key');

expect(isDestructuringAssignment(def, 'Component')).toBe(true);
});

it('fails if name does not match', () => {
const def = parse(`
var { Component } = require('react');
`).get('body', 0, 'declarations', 0, 'id', 'properties', 0, 'key');

expect(isDestructuringAssignment(def, 'Component2')).toBe(false);
});

it('detects destructuring with alias', () => {
const def = parse(`
var { Component: C } = require('react');
`).get('body', 0, 'declarations', 0, 'id', 'properties', 0, 'value');

expect(isDestructuringAssignment(def, 'Component')).toBe(true);
});

it('fails if name does not match with alias', () => {
const def = parse(`
var { Component: C } = require('react');
`).get('body', 0, 'declarations', 0, 'id', 'properties', 0, 'value');

expect(isDestructuringAssignment(def, 'Component2')).toBe(false);
});
});
13 changes: 11 additions & 2 deletions src/utils/__tests__/isReactComponentClass-test.ts
Expand Up @@ -86,17 +86,26 @@ describe('isReactComponentClass', () => {

it('resolves the super class reference', () => {
const def = parse(`
var {Component} = require('react');
var { Component } = require('react');
var C = Component;
class Foo extends C {}
`).get('body', 2);

expect(isReactComponentClass(def, noopImporter)).toBe(true);
});

it('resolves the super class reference with alias', () => {
const def = parse(`
var { Component: C } = require('react');
class Foo extends C {}
`).get('body', 1);

expect(isReactComponentClass(def, noopImporter)).toBe(true);
});

it('does not accept references to other modules', () => {
const def = parse(`
var {Component} = require('FakeReact');
var { Component } = require('FakeReact');
class Foo extends Component {}
`).get('body', 1);

Expand Down
10 changes: 2 additions & 8 deletions src/utils/__tests__/resolveToValue-test.ts
Expand Up @@ -15,15 +15,9 @@ describe('resolveToValue', () => {
['var {foo: {bar: baz}} = bar;', 'baz;'].join('\n'),
);

// Node should be equal to bar.foo.bar
// Resolves to identifier in destructuring
expect(resolveToValue(path, noopImporter)).toEqualASTNode(
builders.memberExpression(
builders.memberExpression(
builders.identifier('bar'),
builders.identifier('foo'),
),
builders.identifier('bar'),
),
builders.identifier('baz'),
);
});

Expand Down
18 changes: 18 additions & 0 deletions src/utils/isDestructuringAssignment.ts
@@ -0,0 +1,18 @@
import { namedTypes as t } from 'ast-types';
import type { NodePath } from 'ast-types/lib/node-path';

/**
* Checks if the input Identifier is part of a destructuring Assignment
* and the name of the property key matches the input name
*/
export default function isDestructuringAssignment(
path: NodePath,
name: string,
): boolean {
return (
t.Identifier.check(path.node) &&
t.Property.check(path.parentPath.node) &&
path.parentPath.node.key.name === name &&
t.ObjectPattern.check(path.parentPath.parentPath.node)
);
}
8 changes: 7 additions & 1 deletion src/utils/isReactBuiltinCall.ts
Expand Up @@ -5,6 +5,7 @@ import resolveToModule from './resolveToModule';
import resolveToValue from './resolveToValue';
import type { Importer } from '../parse';
import type { NodePath } from 'ast-types/lib/node-path';
import isDestructuringAssignment from './isDestructuringAssignment';

/**
* Returns true if the expression is a function call of the form
Expand All @@ -28,7 +29,12 @@ export default function isReactBuiltinCall(
const value = resolveToValue(path.get('callee'), importer);
if (value === path.get('callee')) return false;

if (
// Check if this is a destructuring assignment
// const { x } = require('react')
if (isDestructuringAssignment(value, name)) {
const module = resolveToModule(value, importer);
return Boolean(module && isReactModuleName(module));
} else if (
// `require('react').createElement`
(t.MemberExpression.check(value.node) &&
t.Identifier.check(value.get('property').node) &&
Expand Down
5 changes: 4 additions & 1 deletion src/utils/isReactComponentClass.ts
Expand Up @@ -5,6 +5,7 @@ import resolveToModule from './resolveToModule';
import resolveToValue from './resolveToValue';
import type { Importer } from '../parse';
import type { NodePath } from 'ast-types/lib/node-path';
import isDestructuringAssignment from './isDestructuringAssignment';

function isRenderMethod(node: ASTNode): boolean {
const isProperty = node.type === 'ClassProperty';
Expand Down Expand Up @@ -43,7 +44,9 @@ export default function isReactComponentClass(
const superClass = resolveToValue(path.get('superClass'), importer);
if (
match(superClass.node, { property: { name: 'Component' } }) ||
match(superClass.node, { property: { name: 'PureComponent' } })
match(superClass.node, { property: { name: 'PureComponent' } }) ||
isDestructuringAssignment(superClass, 'Component') ||
isDestructuringAssignment(superClass, 'PureComponent')
) {
const module = resolveToModule(superClass, importer);
if (module && isReactModuleName(module)) {
Expand Down
8 changes: 7 additions & 1 deletion src/utils/resolveToModule.ts
Expand Up @@ -37,9 +37,15 @@ export default function resolveToModule(
if (valuePath !== path) {
return resolveToModule(valuePath, importer);
}
break;
if (!t.Property.check(path.parentPath.node)) {
break;
}
}

// @ts-ignore // fall through
case t.Property.name: // @ts-ignore
case t.ObjectPattern.name:
return resolveToModule(path.parentPath, importer);
// @ts-ignore
case t.ImportDeclaration.name:
return node.source.value;
Expand Down
30 changes: 1 addition & 29 deletions src/utils/resolveToValue.ts
@@ -1,4 +1,4 @@
import { NodePath as Path, builders, namedTypes as t } from 'ast-types';
import { namedTypes as t } from 'ast-types';
import getMemberExpressionRoot from './getMemberExpressionRoot';
import getPropertyValuePath from './getPropertyValuePath';
import { Array as toArray } from './expressionTo';
Expand All @@ -11,28 +11,6 @@ import type { NodePath } from 'ast-types/lib/node-path';
import { Scope } from 'ast-types/lib/scope';
import { Context } from 'ast-types/lib/path-visitor';

function buildMemberExpressionFromPattern(path: NodePath): NodePath | null {
const node = path.node;
if (t.Property.check(node)) {
const objPath = buildMemberExpressionFromPattern(path.parent);
if (objPath) {
return new Path(
builders.memberExpression(
objPath.node,
node.key,
t.Literal.check(node.key),
),
objPath,
);
}
} else if (t.ObjectPattern.check(node)) {
return buildMemberExpressionFromPattern(path.parent);
} else if (t.VariableDeclarator.check(node)) {
return path.get('init');
}
return null;
}

function findScopePath(
paths: NodePath[],
path: NodePath,
Expand Down Expand Up @@ -74,12 +52,6 @@ function findScopePath(
t.TSInterfaceDeclaration.check(parentPath.node)
) {
resultPath = parentPath;
} else if (t.Property.check(parentPath.node)) {
// must be inside a pattern
const memberExpressionPath = buildMemberExpressionFromPattern(parentPath);
if (memberExpressionPath) {
return memberExpressionPath;
}
}

if (resultPath.node !== path.node) {
Expand Down

0 comments on commit 5bcf56c

Please sign in to comment.