Skip to content

Commit 602bfbf

Browse files
committed
fix: do not forward data-layout-page-key
1 parent 5a5fb2b commit 602bfbf

File tree

6 files changed

+59
-14
lines changed

6 files changed

+59
-14
lines changed

.babelrc.json

-3
This file was deleted.

babel.config.js

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
'use strict';
2+
3+
module.exports = (api) => {
4+
api.cache(true);
5+
6+
return {
7+
ignore: process.env.BABEL_ENV ? ['**/*.test.js', '**/__snapshots__', '**/__mocks__', '**/__fixtures__'] : [],
8+
presets: [
9+
['@moxy/babel-preset/lib', { react: true }],
10+
],
11+
};
12+
};

src/LayoutTree.js

+7-5
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import PropTypes from 'prop-types';
33
import memoizeOne from 'memoize-one';
44
import LayoutContext from './util/context';
55
import createFullTree from './util/full-tree';
6-
import { getInitialLayoutTree } from './with-layout';
6+
import { getInitialLayoutTree, isComponentWrapped } from './with-layout';
77

88
const LayoutProvider = LayoutContext.Provider;
99

@@ -48,10 +48,12 @@ export default class LayoutTree extends PureComponent {
4848
const { defaultLayout, Component, pageProps, pageKey, children: render } = this.props;
4949
const { layoutTree } = this.state;
5050

51-
// Note that we use a data attribute to propagate the page key because Component might not be wrapped in WithLayout HOC.
52-
// If we passed a regular `pageKey` property, and if the component spreaded the props, it would throw:
53-
// "React does not recognize the `pageKey` prop on a DOM element"
54-
const page = <Component { ...pageProps } key={ pageKey } data-layout-page-key={ pageKey } />;
51+
// Do not forward pageKey if the component is not wrapped, otherwise it would cause
52+
// an error if props were spreaded into a DOM element:
53+
// "React does not recognize the `pageKey` prop on a DOM element".
54+
const isWrapped = isComponentWrapped(Component);
55+
const page = <Component { ...pageProps } { ...(isWrapped && { pageKey }) } key={ pageKey } />;
56+
5557
const fullTree = createFullTree(layoutTree ?? defaultLayout, page);
5658
const providerValue = this.getProviderValue(Component, pageKey);
5759

src/with-layout.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ const withLayout = (mapLayoutStateToLayoutTree, mapPropsToInitialLayoutState) =>
1616
return (Component) => {
1717
const WithLayout = forwardRef((_props, ref) => {
1818
const { pageKey, props } = useMemo(() => {
19-
const { 'data-layout-page-key': pageKey, ...props } = _props;
19+
const { pageKey, ...props } = _props;
2020

2121
return { pageKey, props };
2222
}, [_props]);
@@ -71,4 +71,6 @@ const withLayout = (mapLayoutStateToLayoutTree, mapPropsToInitialLayoutState) =>
7171

7272
export const getInitialLayoutTree = (Component, pageProps) => Component[getInitialLayoutTreeSymbol]?.(pageProps);
7373

74+
export const isComponentWrapped = (Component) => !!Component[getInitialLayoutTreeSymbol];
75+
7476
export default withLayout;

test/__snapshots__/index.test.js.snap

+24-3
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ exports[`should ignore setLayoutState calls if Component's pageKey is not the ac
5757
>
5858
<main>
5959
<WithLayout(Foo)
60-
data-layout-page-key="foo"
6160
key="foo"
61+
pageKey="foo"
6262
>
6363
<Foo
6464
setLayoutState={[Function]}
@@ -122,6 +122,27 @@ exports[`should ignore setLayoutState calls if page is not the active Component
122122
</LayoutTree>
123123
`;
124124

125+
exports[`should not forward pageKey if component in not wrapped with HOC 1`] = `
126+
<LayoutTree
127+
Component={[Function]}
128+
pageKey="foo"
129+
pageProps={
130+
Object {
131+
"foo": "bar",
132+
}
133+
}
134+
>
135+
<Home
136+
foo="bar"
137+
key="foo"
138+
>
139+
<h1>
140+
Home
141+
</h1>
142+
</Home>
143+
</LayoutTree>
144+
`;
145+
125146
exports[`should render a layout tree correctly based the initial layout state (function) 1`] = `
126147
<LayoutTree
127148
Component={
@@ -367,8 +388,8 @@ exports[`should update the layout tree correctly if pageKey changes 1`] = `
367388
>
368389
<main>
369390
<WithLayout(Home)
370-
data-layout-page-key="foo"
371391
key="foo"
392+
pageKey="foo"
372393
>
373394
<Home
374395
setLayoutState={[Function]}
@@ -399,8 +420,8 @@ exports[`should update the layout tree correctly if pageKey changes 2`] = `
399420
>
400421
<main>
401422
<WithLayout(Home)
402-
data-layout-page-key="bar"
403423
key="bar"
424+
pageKey="bar"
404425
>
405426
<Home
406427
setLayoutState={[Function]}

test/index.test.js

+13-2
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ it('should not inject setLayoutState to pages if it is not needed', () => {
206206
expect(HomeMock).toHaveBeenCalledWith({}, {});
207207
});
208208

209-
it('should fail if layout tree is not unary', () => {
209+
it('should throw if layout tree is not unary', () => {
210210
jest.spyOn(console, 'error').mockImplementation(() => {});
211211

212212
const NewsletterForm = () => <form />;
@@ -242,7 +242,7 @@ it('should allow passing a custom render prop to LayoutTree', () => {
242242
expect(render.mock.results[0].value).toMatchSnapshot();
243243
});
244244

245-
it('should fail if LayoutTree was not rendered', () => {
245+
it('should throw if LayoutTree was not rendered', () => {
246246
jest.spyOn(console, 'error').mockImplementation(() => {});
247247

248248
const EnhancedHome = withLayout(<PrimaryLayout />)(Home);
@@ -316,3 +316,14 @@ it('should ignore setLayoutState calls if Component\'s pageKey is not the active
316316
expect(wrapper).toMatchSnapshot();
317317
expect(mapLayoutStateToLayoutTree).toHaveBeenCalledTimes(1);
318318
});
319+
320+
it('should not forward pageKey if component in not wrapped with HOC', () => {
321+
const wrapper = mount(
322+
<LayoutTree
323+
Component={ Home }
324+
pageProps={ { foo: 'bar' } }
325+
pageKey="foo" />,
326+
);
327+
328+
expect(wrapper).toMatchSnapshot();
329+
});

0 commit comments

Comments
 (0)