Skip to content

Commit 8243f3f

Browse files
rickhanloniirickhanlonii
authored
[bug] Fix component name for Portal and add tests (#32640)
Based off: #32499 While looking into `React.lazy` issues for built-ins, I noticed we already error for `lazy` with build-ins, but we don't have any tests for `getComponentNameFromType` using all the built-ins. This may be something we should handle, but for now we should at least have tests. Here's why: while writing tests, I noticed we check `type` instead of `$$typeof` for portals: https://github.com/facebook/react/blob/9cdf8a99edcfd94d7420835ea663edca04237527/packages/react-reconciler/src/ReactPortal.js#L25-L32 This PR adds tests for all the built-ins and fixes the portal bug. [Commit to review](e068c16)
1 parent df31952 commit 8243f3f

File tree

2 files changed

+277
-3
lines changed

2 files changed

+277
-3
lines changed

‎packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js

Lines changed: 275 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -791,7 +791,7 @@ describe('ReactLazy', () => {
791791
);
792792
});
793793

794-
it('throws with a useful error when wrapping fragment with lazy()', async () => {
794+
it('throws with a useful error when wrapping Fragment with lazy()', async () => {
795795
const BadLazy = lazy(() => fakeImport(React.Fragment));
796796

797797
const root = ReactTestRenderer.create(
@@ -817,6 +817,280 @@ describe('ReactLazy', () => {
817817
);
818818
});
819819

820+
// @gate !fb
821+
it('throws with a useful error when wrapping createPortal with lazy()', async () => {
822+
const ReactDOM = require('react-dom');
823+
const container = document.createElement('div');
824+
const portal = ReactDOM.createPortal(<div />, container);
825+
const BadLazy = lazy(() => fakeImport(portal));
826+
827+
const root = ReactTestRenderer.create(
828+
<Suspense fallback={<Text text="Loading..." />}>
829+
<BadLazy />
830+
</Suspense>,
831+
{
832+
unstable_isConcurrent: true,
833+
},
834+
);
835+
836+
await waitForAll(['Loading...']);
837+
838+
await resolveFakeImport(portal);
839+
root.update(
840+
<Suspense fallback={<Text text="Loading..." />}>
841+
<BadLazy />
842+
</Suspense>,
843+
);
844+
await waitForThrow(
845+
'Element type is invalid. Received a promise that resolves to: Portal. ' +
846+
'Lazy element type must resolve to a class or function.',
847+
);
848+
});
849+
850+
it('throws with a useful error when wrapping Profiler with lazy()', async () => {
851+
const BadLazy = lazy(() => fakeImport(React.Profiler));
852+
853+
const root = ReactTestRenderer.create(
854+
<Suspense fallback={<Text text="Loading..." />}>
855+
<BadLazy />
856+
</Suspense>,
857+
{
858+
unstable_isConcurrent: true,
859+
},
860+
);
861+
862+
await waitForAll(['Loading...']);
863+
864+
await resolveFakeImport(React.Profiler);
865+
root.update(
866+
<Suspense fallback={<Text text="Loading..." />}>
867+
<BadLazy />
868+
</Suspense>,
869+
);
870+
await waitForThrow(
871+
'Element type is invalid. Received a promise that resolves to: Profiler. ' +
872+
'Lazy element type must resolve to a class or function.',
873+
);
874+
});
875+
876+
it('throws with a useful error when wrapping StrictMode with lazy()', async () => {
877+
const BadLazy = lazy(() => fakeImport(React.StrictMode));
878+
879+
const root = ReactTestRenderer.create(
880+
<Suspense fallback={<Text text="Loading..." />}>
881+
<BadLazy />
882+
</Suspense>,
883+
{
884+
unstable_isConcurrent: true,
885+
},
886+
);
887+
888+
await waitForAll(['Loading...']);
889+
890+
await resolveFakeImport(React.StrictMode);
891+
root.update(
892+
<Suspense fallback={<Text text="Loading..." />}>
893+
<BadLazy />
894+
</Suspense>,
895+
);
896+
await waitForThrow(
897+
'Element type is invalid. Received a promise that resolves to: StrictMode. ' +
898+
'Lazy element type must resolve to a class or function.',
899+
);
900+
});
901+
902+
it('throws with a useful error when wrapping Suspense with lazy()', async () => {
903+
const BadLazy = lazy(() => fakeImport(React.Suspense));
904+
905+
const root = ReactTestRenderer.create(
906+
<Suspense fallback={<Text text="Loading..." />}>
907+
<BadLazy />
908+
</Suspense>,
909+
{
910+
unstable_isConcurrent: true,
911+
},
912+
);
913+
914+
await waitForAll(['Loading...']);
915+
916+
await resolveFakeImport(React.Suspense);
917+
root.update(
918+
<Suspense fallback={<Text text="Loading..." />}>
919+
<BadLazy />
920+
</Suspense>,
921+
);
922+
await waitForThrow(
923+
'Element type is invalid. Received a promise that resolves to: Suspense. ' +
924+
'Lazy element type must resolve to a class or function.',
925+
);
926+
});
927+
928+
it('throws with a useful error when wrapping Context with lazy()', async () => {
929+
const Context = React.createContext(null);
930+
const BadLazy = lazy(() => fakeImport(Context));
931+
932+
const root = ReactTestRenderer.create(
933+
<Suspense fallback={<Text text="Loading..." />}>
934+
<BadLazy />
935+
</Suspense>,
936+
{
937+
unstable_isConcurrent: true,
938+
},
939+
);
940+
941+
await waitForAll(['Loading...']);
942+
943+
await resolveFakeImport(Context);
944+
root.update(
945+
<Suspense fallback={<Text text="Loading..." />}>
946+
<BadLazy />
947+
</Suspense>,
948+
);
949+
await waitForThrow(
950+
gate('enableRenderableContext')
951+
? 'Element type is invalid. Received a promise that resolves to: Context.Provider. ' +
952+
'Lazy element type must resolve to a class or function.'
953+
: 'Element type is invalid. Received a promise that resolves to: Context.Consumer. ' +
954+
'Lazy element type must resolve to a class or function.',
955+
);
956+
});
957+
958+
// @gate enableRenderableContext
959+
it('throws with a useful error when wrapping Context.Consumer with lazy()', async () => {
960+
const Context = React.createContext(null);
961+
const BadLazy = lazy(() => fakeImport(Context.Consumer));
962+
963+
const root = ReactTestRenderer.create(
964+
<Suspense fallback={<Text text="Loading..." />}>
965+
<BadLazy />
966+
</Suspense>,
967+
{
968+
unstable_isConcurrent: true,
969+
},
970+
);
971+
972+
await waitForAll(['Loading...']);
973+
974+
await resolveFakeImport(Context.Consumer);
975+
root.update(
976+
<Suspense fallback={<Text text="Loading..." />}>
977+
<BadLazy />
978+
</Suspense>,
979+
);
980+
await waitForThrow(
981+
'Element type is invalid. Received a promise that resolves to: Context.Consumer. ' +
982+
'Lazy element type must resolve to a class or function.',
983+
);
984+
});
985+
986+
// @gate enableSuspenseList
987+
it('throws with a useful error when wrapping SuspenseList with lazy()', async () => {
988+
const BadLazy = lazy(() => fakeImport(React.unstable_SuspenseList));
989+
990+
const root = ReactTestRenderer.create(
991+
<Suspense fallback={<Text text="Loading..." />}>
992+
<BadLazy />
993+
</Suspense>,
994+
{
995+
unstable_isConcurrent: true,
996+
},
997+
);
998+
999+
await waitForAll(['Loading...']);
1000+
1001+
await resolveFakeImport(React.unstable_SuspenseList);
1002+
root.update(
1003+
<Suspense fallback={<Text text="Loading..." />}>
1004+
<BadLazy />
1005+
</Suspense>,
1006+
);
1007+
await waitForThrow(
1008+
'Element type is invalid. Received a promise that resolves to: SuspenseList. ' +
1009+
'Lazy element type must resolve to a class or function.',
1010+
);
1011+
});
1012+
1013+
// @gate enableViewTransition
1014+
it('throws with a useful error when wrapping ViewTransition with lazy()', async () => {
1015+
const BadLazy = lazy(() => fakeImport(React.unstable_ViewTransition));
1016+
1017+
const root = ReactTestRenderer.create(
1018+
<Suspense fallback={<Text text="Loading..." />}>
1019+
<BadLazy />
1020+
</Suspense>,
1021+
{
1022+
unstable_isConcurrent: true,
1023+
},
1024+
);
1025+
1026+
await waitForAll(['Loading...']);
1027+
1028+
await resolveFakeImport(React.unstable_ViewTransition);
1029+
root.update(
1030+
<Suspense fallback={<Text text="Loading..." />}>
1031+
<BadLazy />
1032+
</Suspense>,
1033+
);
1034+
await waitForThrow(
1035+
'Element type is invalid. Received a promise that resolves to: ViewTransition. ' +
1036+
'Lazy element type must resolve to a class or function.',
1037+
);
1038+
});
1039+
1040+
// @gate enableActivity
1041+
it('throws with a useful error when wrapping Activity with lazy()', async () => {
1042+
const BadLazy = lazy(() => fakeImport(React.unstable_Activity));
1043+
1044+
const root = ReactTestRenderer.create(
1045+
<Suspense fallback={<Text text="Loading..." />}>
1046+
<BadLazy />
1047+
</Suspense>,
1048+
{
1049+
unstable_isConcurrent: true,
1050+
},
1051+
);
1052+
1053+
await waitForAll(['Loading...']);
1054+
1055+
await resolveFakeImport(React.unstable_Activity);
1056+
root.update(
1057+
<Suspense fallback={<Text text="Loading..." />}>
1058+
<BadLazy />
1059+
</Suspense>,
1060+
);
1061+
await waitForThrow(
1062+
'Element type is invalid. Received a promise that resolves to: Activity. ' +
1063+
'Lazy element type must resolve to a class or function.',
1064+
);
1065+
});
1066+
1067+
// @gate enableTransitionTracing
1068+
it('throws with a useful error when wrapping TracingMarker with lazy()', async () => {
1069+
const BadLazy = lazy(() => fakeImport(React.unstable_TracingMarker));
1070+
1071+
const root = ReactTestRenderer.create(
1072+
<Suspense fallback={<Text text="Loading..." />}>
1073+
<BadLazy />
1074+
</Suspense>,
1075+
{
1076+
unstable_isConcurrent: true,
1077+
},
1078+
);
1079+
1080+
await waitForAll(['Loading...']);
1081+
1082+
await resolveFakeImport(React.unstable_TracingMarker);
1083+
root.update(
1084+
<Suspense fallback={<Text text="Loading..." />}>
1085+
<BadLazy />
1086+
</Suspense>,
1087+
);
1088+
await waitForThrow(
1089+
'Element type is invalid. Received a promise that resolves to: TracingMarker. ' +
1090+
'Lazy element type must resolve to a class or function.',
1091+
);
1092+
});
1093+
8201094
it('throws with a useful error when wrapping lazy() multiple times', async () => {
8211095
const Lazy1 = lazy(() => fakeImport(Text));
8221096
const Lazy2 = lazy(() => fakeImport(Lazy1));

‎packages/shared/getComponentNameFromType.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,6 @@ export default function getComponentNameFromType(type: mixed): string | null {
7474
switch (type) {
7575
case REACT_FRAGMENT_TYPE:
7676
return 'Fragment';
77-
case REACT_PORTAL_TYPE:
78-
return 'Portal';
7977
case REACT_PROFILER_TYPE:
8078
return 'Profiler';
8179
case REACT_STRICT_MODE_TYPE:
@@ -106,6 +104,8 @@ export default function getComponentNameFromType(type: mixed): string | null {
106104
}
107105
}
108106
switch (type.$$typeof) {
107+
case REACT_PORTAL_TYPE:
108+
return 'Portal';
109109
case REACT_PROVIDER_TYPE:
110110
if (enableRenderableContext) {
111111
return null;

0 commit comments

Comments
 (0)