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

feat: support more element types #617

Merged
merged 6 commits into from
Sep 28, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@
"smoke": "node tests/smoke/run"
},
"lint-staged": {
"*.js": [
"prettier --write \"**/*.{js,json}\"",
"git add"
]
"*.js": ["prettier --write \"**/*.{js,json}\"", "git add"]
},
"author": {
"name": "Algolia, Inc.",
Expand Down Expand Up @@ -72,7 +69,6 @@
"react-test-renderer": "16.14.0",
"rollup": "1.32.1",
"rollup-plugin-babel": "4.4.0",
"rollup-plugin-commonjs": "10.1.0",
"rollup-plugin-node-builtins": "2.1.2",
"rollup-plugin-node-globals": "1.4.0",
"rollup-plugin-node-resolve": "5.2.0",
Expand All @@ -84,11 +80,10 @@
},
"dependencies": {
"@base2/pretty-print-object": "1.0.0",
"is-plain-object": "3.0.1"
"is-plain-object": "3.0.1",
"react-is": "^17.0.2"
},
"jest": {
"setupFilesAfterEnv": [
"<rootDir>tests/setupTests.js"
]
"setupFilesAfterEnv": ["<rootDir>tests/setupTests.js"]
}
}
15 changes: 5 additions & 10 deletions rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import babel from 'rollup-plugin-babel';
import commonjs from 'rollup-plugin-commonjs';
import resolve from 'rollup-plugin-node-resolve';
import builtins from 'rollup-plugin-node-builtins';
import globals from 'rollup-plugin-node-globals';
import pkg from './package.json';

const extractPackagePeerDependencies = () =>
Object.keys(pkg.peerDependencies) || [];
const extractExternals = () => [
...Object.keys(pkg.dependencies || {}),
...Object.keys(pkg.peerDependencies || {}),
];

export default {
input: 'src/index.js',
Expand All @@ -22,7 +23,7 @@ export default {
sourcemap: true,
},
],
external: extractPackagePeerDependencies(),
external: extractExternals(),
plugins: [
babel({
babelrc: false,
Expand All @@ -36,12 +37,6 @@ export default {
resolve({
mainFields: ['module', 'main', 'jsnext', 'browser'],
}),
commonjs({
sourceMap: true,
namedExports: {
react: ['isValidElement'],
},
}),
Comment on lines -39 to -44
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why you have to change that in the context of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commonjs plugin is only needed when you actually want to bundle a commonjs dependency - you don't really want that with react since you are leaving it as external (like you should), so this was actually a "dead"/redundant config

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's improve this!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, could you be more specific? maybe I've explained this poorly - so let me rephrase: the removed code that was not actually used and it wasn't doing anything here. And how this config now is defined is correct - from my point of view this change is "sound" and correct. There is no regression here so this can be merged "as is". Please let me know if this addresses your concerns - if not I can try to explain it further and in more detail.

Copy link
Collaborator

@armandabric armandabric Sep 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ho! Sorry I just wanted to said Let's change this! in the meaning of accepting your change with pleasure. Sorry for the wrong wording 🙊

globals(),
builtins(),
],
Expand Down
170 changes: 170 additions & 0 deletions src/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1160,4 +1160,174 @@ describe('reactElementToJSXString(ReactElement)', () => {
}
mount(<App />);
});

it('should use inferred function name as display name for `forwardRef` element', () => {
const Tag = React.forwardRef(function Tag({ text }, ref) {
return <span ref={ref}>{text}</span>;
});
expect(reactElementToJSXString(<Tag text="some label" />)).toEqual(
`<Tag text="some label" />`
);
});

it('should use `displayName` instead of inferred function name as display name for `forwardRef` element', () => {
const Tag = React.forwardRef(function Tag({ text }, ref) {
return <span ref={ref}>{text}</span>;
});
Tag.displayName = 'MyTag';
expect(reactElementToJSXString(<Tag text="some label" />)).toEqual(
`<MyTag text="some label" />`
);
});

it('should use inferred function name as display name for `memo` element', () => {
const Tag = React.memo(function Tag({ text }) {
return <span>{text}</span>;
});
expect(reactElementToJSXString(<Tag text="some label" />)).toEqual(
`<Tag text="some label" />`
);
});

it('should use `displayName` instead of inferred function name as display name for `memo` element', () => {
const Tag = React.memo(function Tag({ text }) {
return <span>{text}</span>;
});
Tag.displayName = 'MyTag';
expect(reactElementToJSXString(<Tag text="some label" />)).toEqual(
`<MyTag text="some label" />`
);
});

it('should use inferred function name as display name for a `forwardRef` wrapped in `memo`', () => {
const Tag = React.memo(
React.forwardRef(function Tag({ text }, ref) {
return <span ref={ref}>{text}</span>;
})
);
expect(reactElementToJSXString(<Tag text="some label" />)).toEqual(
`<Tag text="some label" />`
);
});

it('should use inferred function name as display name for a component wrapped in `memo` multiple times', () => {
const Tag = React.memo(
React.memo(
React.memo(function Tag({ text }) {
return <span>{text}</span>;
})
)
);
expect(reactElementToJSXString(<Tag text="some label" />)).toEqual(
`<Tag text="some label" />`
);
});

it('should stringify `StrictMode` correctly', () => {
const App = () => null;

expect(
reactElementToJSXString(
<React.StrictMode>
<App />
</React.StrictMode>
)
).toEqual(`<StrictMode>
<App />
</StrictMode>`);
});

it('should stringify `Suspense` correctly', () => {
const Spinner = () => null;
const ProfilePage = () => null;

expect(
reactElementToJSXString(
<React.Suspense fallback={<Spinner />}>
<ProfilePage />
</React.Suspense>
)
).toEqual(`<Suspense fallback={<Spinner />}>
<ProfilePage />
</Suspense>`);
});

it('should stringify `Profiler` correctly', () => {
const Navigation = () => null;

expect(
reactElementToJSXString(
<React.Profiler id="Navigation" onRender={() => {}}>
<Navigation />
</React.Profiler>
)
).toEqual(`<Profiler
id="Navigation"
onRender={function noRefCheck() {}}
>
<Navigation />
</Profiler>`);
});

it('should stringify `Contex.Provider` correctly', () => {
const Ctx = React.createContext();
const App = () => {};

expect(
reactElementToJSXString(
<Ctx.Provider value={null}>
<App />
</Ctx.Provider>
)
).toEqual(`<Context.Provider value={null}>
<App />
</Context.Provider>`);
});

it('should stringify `Contex.Provider` with `displayName` correctly', () => {
const Ctx = React.createContext();
Ctx.displayName = 'MyCtx';

const App = () => {};

expect(
reactElementToJSXString(
<Ctx.Provider value={null}>
<App />
</Ctx.Provider>
)
).toEqual(`<MyCtx.Provider value={null}>
<App />
</MyCtx.Provider>`);
});

it('should stringify `Contex.Consumer` correctly', () => {
const Ctx = React.createContext();
const Button = () => null;

expect(
reactElementToJSXString(
<Ctx.Consumer>{theme => <Button theme={theme} />}</Ctx.Consumer>
)
).toEqual(`<Context.Consumer />`);
});

it('should stringify `Contex.Consumer` with `displayName` correctly', () => {
const Ctx = React.createContext();
Ctx.displayName = 'MyCtx';

const Button = () => null;

expect(
reactElementToJSXString(
<Ctx.Consumer>{theme => <Button theme={theme} />}</Ctx.Consumer>
)
).toEqual(`<MyCtx.Consumer />`);
});

it('should stringify `lazy` component correctly', () => {
const Lazy = React.lazy(() => Promise.resolve(() => {}));

expect(reactElementToJSXString(<Lazy />)).toEqual(`<Lazy />`);
});
});
68 changes: 62 additions & 6 deletions src/parser/parseReactElement.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
/* @flow */

import React, { type Element as ReactElement, Fragment } from 'react';
import {
ForwardRef,
isContextConsumer,
isContextProvider,
isForwardRef,
isLazy,
isMemo,
isProfiler,
isStrictMode,
isSuspense,
Memo,
} from 'react-is';
import type { Options } from './../options';
import {
createStringTreeNode,
Expand All @@ -12,12 +24,56 @@ import type { TreeNode } from './../tree';

const supportFragment = Boolean(Fragment);

const getReactElementDisplayName = (element: ReactElement<*>): string =>
element.type.displayName ||
(element.type.name !== '_default' ? element.type.name : null) || // function name
(typeof element.type === 'function' // function without a name, you should provide one
? 'No Display Name'
: element.type);
const getFunctionTypeName = (functionType): string => {
if (!functionType.name || functionType.name === '_default') {
return 'No Display Name';
}
return functionType.name;
};

const getWrappedComponentDisplayName = (Component: *): string => {
switch (true) {
case Boolean(Component.displayName):
return Component.displayName;
case Component.$$typeof === Memo:
return getWrappedComponentDisplayName(Component.type);
case Component.$$typeof === ForwardRef:
return getWrappedComponentDisplayName(Component.render);
default:
return getFunctionTypeName(Component);
}
};

// heavily inspired by:
// https://github.com/facebook/react/blob/3746eaf985dd92f8aa5f5658941d07b6b855e9d9/packages/react-devtools-shared/src/backend/renderer.js#L399-L496
const getReactElementDisplayName = (element: ReactElement<*>): string => {
switch (true) {
case typeof element.type === 'string':
return element.type;
case typeof element.type === 'function':
if (element.type.displayName) {
return element.type.displayName;
}
return getFunctionTypeName(element.type);
case isForwardRef(element):
case isMemo(element):
return getWrappedComponentDisplayName(element.type);
case isContextConsumer(element):
return `${element.type._context.displayName || 'Context'}.Consumer`;
case isContextProvider(element):
return `${element.type._context.displayName || 'Context'}.Provider`;
case isLazy(element):
return 'Lazy';
case isProfiler(element):
return 'Profiler';
case isStrictMode(element):
return 'StrictMode';
case isSuspense(element):
return 'Suspense';
default:
return 'UnknownElementType';
}
};

const noChildren = (propsValue, propName) => propName !== 'children';

Expand Down
Loading