-
Notifications
You must be signed in to change notification settings - Fork 49.8k
Description
Summary
react-refresh uses an unsafe heuristic to recognize React components.
If the input is a Proxy instance that throws in its handler, react-refresh will crash. In my case, I've been using react-refresh through react-refresh-webpack-plugin.
Steps to reproduce
Export this, and set up react-refresh-webpack-plugin:
export default new Proxy({}, {
get(target, property) {
throw new Error();
}
});Why this happens
Currently, react-refresh attempts to access type.$$typeof directly (in isLikelyComponentType):
react/packages/react-refresh/src/ReactFreshRuntime.js
Lines 677 to 687 in d95c493
| case 'object': { | |
| if (type != null) { | |
| switch (type.$$typeof) { | |
| case REACT_FORWARD_REF_TYPE: | |
| case REACT_MEMO_TYPE: | |
| // Definitely React components. | |
| return true; | |
| default: | |
| return false; | |
| } | |
| } |
Accessing $$typeof like that is dangerous and should be wrapped in a try-catch block.
I would like to fix this in a PR, but in doing so, I found yet another instance where the same unsafe heuristic is used (in register):
react/packages/react-refresh/src/ReactFreshRuntime.js
Lines 323 to 333 in d95c493
| // Visit inner types because we might not have registered them. | |
| if (typeof type === 'object' && type !== null) { | |
| switch (type.$$typeof) { | |
| case REACT_FORWARD_REF_TYPE: | |
| register(type.render, id + '$render'); | |
| break; | |
| case REACT_MEMO_TYPE: | |
| register(type.type, id + '$type'); | |
| break; | |
| } | |
| } |
I'm unsure if the occurrence in the register function should have the same safety check, or if checking in isLikelyComponentType suffices.
@gaearon: Would you happen to know?