Skip to content

Commit

Permalink
Remove match utility and cleanup code (#744)
Browse files Browse the repository at this point in the history
  • Loading branch information
danez authored Jan 24, 2023
1 parent 5215bab commit e956802
Show file tree
Hide file tree
Showing 17 changed files with 136 additions and 126 deletions.
9 changes: 9 additions & 0 deletions .changeset/lazy-lions-dress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'react-docgen': major
---

Remove match utility.

The utility can be replaced by babel helpers and is not needed anymore. Also
using explicit checks like `path.isMemberExpression()` is better for type safety
and catching potential bugs.
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ const explodedVisitors = visitors.explode<TraverseState>({
if (
binding &&
left.isMemberExpression() &&
left.get('object').isIdentifier() &&
(left.node.object as Identifier).name === name &&
left.get('object').isIdentifier({ name }) &&
binding.scope === scope &&
resolveToValue(assignmentPath.get('right')).isFunction()
) {
Expand Down
11 changes: 5 additions & 6 deletions packages/react-docgen/src/importer/makeFsImporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { dirname, extname } from 'path';
import fs from 'fs';
import type { NodePath } from '@babel/traverse';
import { visitors } from '@babel/traverse';
import type { ExportSpecifier, Identifier, ObjectProperty } from '@babel/types';
import type { ExportSpecifier, ObjectProperty } from '@babel/types';
import type { Importer, ImportPath } from './index.js';
import type FileState from '../FileState.js';
import { resolveObjectPatternPropertyToValue } from '../utils/index.js';
Expand Down Expand Up @@ -164,7 +164,7 @@ export default function makeFsImporter(
const id = declPath.get('id');
const init = declPath.get('init');

if (id.isIdentifier() && id.node.name === name && init.hasNode()) {
if (id.isIdentifier({ name }) && init.hasNode()) {
// export const/var a = <init>

state.resultPath = init;
Expand All @@ -177,7 +177,7 @@ export default function makeFsImporter(
if (prop.isObjectProperty()) {
const value = prop.get('value');

return value.isIdentifier() && value.node.name === name;
return value.isIdentifier({ name });
}
// We don't handle RestElement here yet as complicated

Expand All @@ -197,8 +197,7 @@ export default function makeFsImporter(
} else if (
declaration.hasNode() &&
declaration.has('id') &&
(declaration.get('id') as NodePath).isIdentifier() &&
(declaration.get('id') as NodePath<Identifier>).node.name === name
(declaration.get('id') as NodePath).isIdentifier({ name })
) {
// export function/class/type/interface/enum ...

Expand All @@ -212,7 +211,7 @@ export default function makeFsImporter(
}
const exported = specifierPath.get('exported');

if (exported.isIdentifier() && exported.node.name === name) {
if (exported.isIdentifier({ name })) {
// export ... from ''
if (path.has('source')) {
const local = specifierPath.isExportSpecifier()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { parse } from '../../../tests/utils';
import isReactChildrenElementCall from '../isReactChildrenElementCall.js';
import { describe, expect, test } from 'vitest';

describe('isReactChildrenElementCall', () => {
describe('true', () => {
test('React.Children.map', () => {
const def = parse.expressionLast(`
var React = require("React");
React.Children.map(() => {});
`);

expect(isReactChildrenElementCall(def)).toBe(true);
});

test('React.Children.only', () => {
const def = parse.expressionLast(`
var React = require("React");
React.Children.only(() => {});
`);

expect(isReactChildrenElementCall(def)).toBe(true);
});
});
describe('false', () => {
test('not call expression', () => {
const def = parse.expressionLast(`
var React = require("React");
React.Children.map;
`);

expect(isReactChildrenElementCall(def)).toBe(false);
});

test('not MemberExpression', () => {
const def = parse.expressionLast(`
var React = require("React");
map();
`);

expect(isReactChildrenElementCall(def)).toBe(false);
});

test('not only or map', () => {
const def = parse.expressionLast(`
var React = require("React");
React.Children.abc(() => {});
`);

expect(isReactChildrenElementCall(def)).toBe(false);
});

test('not double MemberExpression', () => {
const def = parse.expressionLast(`
var React = require("React");
Children.map(() => {});
`);

expect(isReactChildrenElementCall(def)).toBe(false);
});

test('not Children', () => {
const def = parse.expressionLast(`
var React = require("React");
React.Parent.map(() => {});
`);

expect(isReactChildrenElementCall(def)).toBe(false);
});

test('not react module', () => {
const def = parse.expressionLast(`
var React = require("test");
React.Children.map(() => {});
`);

expect(isReactChildrenElementCall(def)).toBe(false);
});
});
});
36 changes: 0 additions & 36 deletions packages/react-docgen/src/utils/__tests__/match-test.ts

This file was deleted.

10 changes: 3 additions & 7 deletions packages/react-docgen/src/utils/getFlowType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,7 @@ function handleGenericTypeAnnotation(
const id = path.get('id');
const typeParameters = path.get('typeParameters');

if (
id.isIdentifier() &&
id.node.name === '$Keys' &&
typeParameters.hasNode()
) {
if (id.isIdentifier({ name: '$Keys' }) && typeParameters.hasNode()) {
return handleKeysHelper(path);
}

Expand All @@ -137,7 +133,7 @@ function handleGenericTypeAnnotation(
if (id.isQualifiedTypeIdentifier()) {
const qualification = id.get('qualification');

if (qualification.isIdentifier() && qualification.node.name === 'React') {
if (qualification.isIdentifier({ name: 'React' })) {
type = {
name: `${qualification.node.name}${id.node.id.name}`,
raw: printValue(id),
Expand Down Expand Up @@ -391,7 +387,7 @@ function getFlowTypeWithResolvedTypes(

const isTypeAlias = parent.isTypeAlias();

// When we see a typealias mark it as visited so that the next
// When we see a TypeAlias mark it as visited so that the next
// call of this function does not run into an endless loop
if (isTypeAlias) {
if (visitedTypes[parent.node.id.name] === true) {
Expand Down
6 changes: 1 addition & 5 deletions packages/react-docgen/src/utils/getTSType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,7 @@ function handleTSTypeReference(
const left = typeName.get('left');
const right = typeName.get('right');

if (
left.isIdentifier() &&
left.node.name === 'React' &&
right.isIdentifier()
) {
if (left.isIdentifier({ name: 'React' }) && right.isIdentifier()) {
type = {
name: `${left.node.name}${right.node.name}`,
raw: printValue(typeName),
Expand Down
1 change: 0 additions & 1 deletion packages/react-docgen/src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ export { default as isReactModuleName } from './isReactModuleName.js';
export { default as isRequiredPropType } from './isRequiredPropType.js';
export { default as isStatelessComponent } from './isStatelessComponent.js';
export { default as isUnreachableFlowType } from './isUnreachableFlowType.js';
export { default as match } from './match.js';
export { default as normalizeClassDefinition } from './normalizeClassDefinition.js';
export { default as parseJsDoc } from './parseJsDoc.js';
export { default as postProcessDocumentation } from './postProcessDocumentation.js';
Expand Down
6 changes: 1 addition & 5 deletions packages/react-docgen/src/utils/isDestructuringAssignment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,5 @@ export default function isDestructuringAssignment(

const id = path.get('key');

return (
id.isIdentifier() &&
id.node.name === name &&
path.parentPath.isObjectPattern()
);
return id.isIdentifier({ name }) && path.parentPath.isObjectPattern();
}
24 changes: 13 additions & 11 deletions packages/react-docgen/src/utils/isReactBuiltinCall.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import isReactModuleName from './isReactModuleName.js';
import match from './match.js';
import resolveToModule from './resolveToModule.js';
import resolveToValue from './resolveToValue.js';
import isDestructuringAssignment from './isDestructuringAssignment.js';
import type { NodePath } from '@babel/traverse';
import type { CallExpression, MemberExpression } from '@babel/types';
import type { CallExpression } from '@babel/types';

function isNamedMemberExpression(value: NodePath, name: string): boolean {
if (!value.isMemberExpression()) {
Expand Down Expand Up @@ -33,8 +32,8 @@ function isNamedImportDeclaration(
const local = specifier.get('local');

return (
((imported.isIdentifier() && imported.node.name === name) ||
(imported.isStringLiteral() && imported.node.value === name)) &&
(imported.isIdentifier({ name }) ||
imported.isStringLiteral({ value: name })) &&
local.node.name === callee.node.name
);
});
Expand All @@ -53,17 +52,20 @@ export default function isReactBuiltinCall(
}

if (path.isCallExpression()) {
if (match(path.node, { callee: { property: { name } } })) {
const module = resolveToModule(
(path.get('callee') as NodePath<MemberExpression>).get('object'),
);
const callee = path.get('callee');

if (
callee.isMemberExpression() &&
callee.get('property').isIdentifier({ name })
) {
const module = resolveToModule(callee.get('object'));

return Boolean(module && isReactModuleName(module));
}

const value = resolveToValue(path.get('callee'));
const value = resolveToValue(callee);

if (value === path.get('callee')) {
if (value === callee) {
return false;
}

Expand All @@ -73,7 +75,7 @@ export default function isReactBuiltinCall(
// `require('react').createElement`
isNamedMemberExpression(value, name) ||
// `import { createElement } from 'react'`
isNamedImportDeclaration(value, path.get('callee'), name)
isNamedImportDeclaration(value, callee, name)
) {
const module = resolveToModule(value);

Expand Down
26 changes: 15 additions & 11 deletions packages/react-docgen/src/utils/isReactChildrenElementCall.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,36 @@
import type { NodePath } from '@babel/traverse';
import type { MemberExpression } from '@babel/types';
import isReactModuleName from './isReactModuleName.js';
import match from './match.js';
import resolveToModule from './resolveToModule.js';

// TODO unit tests

/**
* Returns true if the expression is a function call of the form
* `React.Children.only(...)`.
* `React.Children.only(...)` or `React.Children.map(...)`.
*/
export default function isReactChildrenElementCall(path: NodePath): boolean {
if (path.isExpressionStatement()) {
path = path.get('expression');
}

if (!path.isCallExpression()) {
return false;
}

const callee = path.get('callee');

if (
!match(path.node, { callee: { property: { name: 'only' } } }) &&
!match(path.node, { callee: { property: { name: 'map' } } })
!callee.isMemberExpression() ||
(!callee.get('property').isIdentifier({ name: 'only' }) &&
!callee.get('property').isIdentifier({ name: 'map' }))
) {
return false;
}

const calleeObj = (path.get('callee') as NodePath<MemberExpression>).get(
'object',
);
const calleeObj = callee.get('object');

if (!match(calleeObj.node, { property: { name: 'Children' } })) {
if (
!calleeObj.isMemberExpression() ||
!calleeObj.get('property').isIdentifier({ name: 'Children' })
) {
return false;
}

Expand Down
6 changes: 2 additions & 4 deletions packages/react-docgen/src/utils/isRequiredPropType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ import getMembers from '../utils/getMembers.js';
export default function isRequiredPropType(path: NodePath<Node>): boolean {
return getMembers(path).some(
({ computed, path: memberPath }) =>
(!computed &&
memberPath.isIdentifier() &&
memberPath.node.name === 'isRequired') ||
(memberPath.isStringLiteral() && memberPath.node.value === 'isRequired'),
(!computed && memberPath.isIdentifier({ name: 'isRequired' })) ||
memberPath.isStringLiteral({ value: 'isRequired' }),
);
}
28 changes: 0 additions & 28 deletions packages/react-docgen/src/utils/match.ts

This file was deleted.

Loading

0 comments on commit e956802

Please sign in to comment.