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

Remove match utility and cleanup code #744

Merged
merged 1 commit into from
Jan 24, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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