-
Notifications
You must be signed in to change notification settings - Fork 30
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
feat(packages/sui-studio): Add new param to use explicit import #1460
base: master
Are you sure you want to change the base?
Conversation
816fc6b
to
f4f84d4
Compare
@@ -52,6 +53,7 @@ const componentInPascal = toPascalCase( | |||
const COMPONENT_DIR = `/components/${category}/${component}/` | |||
const COMPONENT_PATH = `${BASE_DIR}${COMPONENT_DIR}` | |||
const COMPONENT_ENTRY_JS_POINT_FILE = `${COMPONENT_PATH}src/index.js` | |||
const COMPONENT_JS_POINT_FILE = `${COMPONENT_PATH}src/${componentInPascal}.js` |
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.
Maybe a more descriptive name would help, something like EXPLICIT_COMPONENT_JS_POINT_FILE
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.
Agree! 👌 Thanks Javi!
const explicitImportTemplate = `import ${componentInPascal} from './${componentInPascal}.js' | ||
|
||
export default ${componentInPascal} | ||
` |
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.
Maybe we can go with the shorter version that mixes the rexport of default and named exports.
const explicitImportTemplate = `import ${componentInPascal} from './${componentInPascal}.js' | |
export default ${componentInPascal} | |
` | |
const explicitImportTemplate = `export {default} from './${componentInPascal}.js' | |
` |
Then if we want to export some named (for example: foo and bar) things inside the component would be much shorter:
const explicitImportTemplate =
export {default, foo, bar} from './${componentInPascal}.js'`
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 is a proposal in stage 1, I checked and we use @babel/plugin-proposal-export-default-from
to support it. If we're sure our infra will keep it, I'm ok with the change, but just want to double check 🙂
If this is the new way of generating component, maybe we should create a script to modify also the already created components, so we just have one standard way of creating components. What do you think? |
Yeah, makes sense! And we'll need to migrate the rest of components to actually solve our pain. Will work on it 👌 Thanks @arnau-rius! |
In case it gives you some guidance, here you can find two migrations that previously had been applied to the sui-studio, for cases similar to this one. https://github.com/SUI-Components/sui/tree/master/packages/sui-studio/bin/migrations
Maybe would be interesting to check that all marketplaces are OK with this new structure before coding/planning a migration. 👍🏽 |
Description
This PR adds a new option
--explicit
to generate a component in a file with its same name and an index re-export, this makes searching easier while keeping cleaner imports.Beta: @s-ui/[email protected]
Feedback is much appreciated 🙂