-
-
Notifications
You must be signed in to change notification settings - Fork 361
[React] Improve error handling in resolveReactComponent
#2006
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
[React] Improve error handling in resolveReactComponent
#2006
Conversation
53f7119
to
a312fee
Compare
Alternatively function const importAllReactComponents = (r: __WebpackModuleApi.RequireContext) => {
- r.keys().forEach((key) => (reactControllers[key] = r(key).default));
+ r.keys().forEach((key) => {
+ if (r(key).default !== undefined) {
+ reactControllers[key] = r(key).default;
+ }
+ });
}; However, with these changes, it will be more difficult to determine if a module or default export is missing. |
throw new Error( | ||
`React controller "${name}" could not be resolved. Ensure the module exports the controller as default.` | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
throw new Error( | |
`React controller "${name}" could not be resolved. Ensure the module exports the controller as default.` | |
); | |
throw new Error(`React controller "${name}" could not be resolved. Ensure the module exports the controller as default.`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran biome check
and applied the suggested formatter changes
Hi @teklakct ! Thanks for this Could you rebuild the JS dist files ? (that's the reason of the CI failure) From the root of the monorepo: (you may need to rebase before) |
bb23f4a
to
b7dba4b
Compare
📊 Packages dist files size differenceThanks for the PR! Here is the difference in size of the packages dist files between the base branch and the PR.
|
@smnandre sorry for the delay. I must have missed the notification earlier. |
@teklakct don't be :) I also lost tracking of this PR to be honest! Seems good to me ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this, and sorry for the delay.
Do you think we could do the same for UX Vue and UX Svelte?
resolveReactComponent
b7dba4b
to
9e36018
Compare
Thank you @teklakct. |
WHAT
Improve error handling and readability in
window.resolveReactComponent
functionWith those changes, when resolving a module with no default export, an error message with a hint will be shown:
WHY
I found that when someone creates a module without a default export then the error message is quite confusing.
For example:
Whenever there will be an element like this
An error like below in console appear:
Which is not exactly true. The module
NoDefaultExportComponent
was found but there is no required default export.