Support shorthand scoped templates #8298
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
kevin940726 requested review from amyrlam, ianschmitz, iansu, mrmckeb and petetnt as code owners 
mrmckeb reviewed Jan 9, 2020
There was a problem hiding this comment.
This looks great @kevin940726! Are you able to add any tests for this?
| @mrmckeb Sure! I was trying to do so too, but couldn’t find an appropriate place to put those tests. Should I create e2e tests? Any recommended example? |
| You could create an E2E test... but those are quite flimsy right now. Perhaps for now we'll leave it, and we'll cover this as we rewrite tests towards v4. |
mrmckeb reviewed Jan 9, 2020
Comment on lines 648 to 650
| // @SCOPE/cra-template | ||
| // cra-template-NAME | ||
| // @SCOPE/cra-template-NAME |
Co-Authored-By: Brody McKee <mrmckeb@users.noreply.github.com>
kevin940726 force-pushed the scope-template branch from 58d7580 to cb007f4 Compare | @mrmckeb That definitely makes them clearer 👍! Just updated :) |
mrmckeb approved these changes Jan 9, 2020
| I've tagged this for 3.3.1, as I don't think it's worth delaying this for v3.4 - and we could argue that it is a bug that it doesn't work this way already. |
heyimalex approved these changes Jan 9, 2020
| Thanks @heyimalex! Merging now @kevin940726 - great work. |
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Continue from #7991. Allow
@scopeshorthand for template package name.The motivation comes from storybookjs/storybook#9327 (comment), where when library authors want to provide cra template under their scoped name, typing
--template @storybook/cra-templateevery time seems tedious. Instead, it'd be better if the users can just type--template @storybookto use the template@storybook/cra-template. Since@is not a valid character for npm package name, it should be backward-compatible.Tests
To list all the possible combinations.
--template cra-templatecra-template--template cra-template-typescriptcra-template-typescript--template typescriptcra-template-typescript--template @scope/cra-template-typescript@scope/cra-template-typescript--template @scope/typescript@scope/cra-template-typescript--template @scope(Added)@scope/cra-templateThis PR also fixes a bug (?) when users provided
--template cra-templateswould still downloadcra-templateswhile it should becra-template-cra-templates, as templates should be prefixed bycra-template-, judging from the doc. We can instead allow the other way around though if that makes more sense.Pinging @mrmckeb as we mentioned it in the issue before :)