Support shorthand scoped templates #8298
Merged
Conversation
This file contains 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. |
# for free
to subscribe to this conversation on GitHub.
Already have an account?
#.
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 :)