-
Notifications
You must be signed in to change notification settings - Fork 27k
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
Dynamic imports with named exports #3590
Dynamic imports with named exports #3590
Conversation
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.
Looks good. Just allows ability to have a higher level of specificity.
Can we add to readme this additional functionality?
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.
This PR looks great.
Try to do these:
- have a look at the suggestions I made
- document this feature in the README
- add a test case for this case
@@ -31,7 +31,7 @@ export default function dynamicComponent (p, o) { | |||
|
|||
this.LoadingComponent = options.loading ? options.loading : () => (<p>loading...</p>) | |||
this.ssr = options.ssr === false ? options.ssr : true | |||
|
|||
this.compName = options.component ? options.component : undefined |
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.
We don't need this expression. Use options.componentName
instead. It's clear and does the job.
@@ -55,7 +55,7 @@ export default function dynamicComponent (p, o) { | |||
|
|||
loadComponent () { | |||
promise.then((m) => { | |||
const AsyncComponent = m.default || m | |||
const AsyncComponent = this.compName ? m[this.compName] : m.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.
Add the m.default || m
part as well.
Extracting exports based on string keys is non-standard and impossible to type with TS/Flow (which hints at a bad pattern IMHO). Making standard Related #2940 |
@herrstucki is actually right, it should be like |
This pull request is a feature allowing dynamic imports to import specific components not just default ones from the module.
The current code only allows default exports to be imported to the outer world.
Usage should look like:
dynamic(import('./moduleName'), { component: 'ComponentName', });
By this, it will import only 'ComponentName' from the module. It will be useful for structures which are having multiple exports from a single file.