Skip to content
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

Generate .tsx stories in typescript projects #5050

Closed
shilman opened this issue Dec 19, 2018 · 7 comments
Closed

Generate .tsx stories in typescript projects #5050

shilman opened this issue Dec 19, 2018 · 7 comments

Comments

@shilman
Copy link
Member

shilman commented Dec 19, 2018

Splitting out "generate tsx stories" as a feature request out of #5028

What

sb init has a --story-format option, which can be csf | csf-ts | mdx. csf-ts templates exist for a few of the different project types. We should have csf-ts templates for all of the different project types.

The default format is csf. But if the user doesn't specify a format, and sb init detects that it's a typescript project, it should use the csf-ts format. It looks like a dependency or devDependency on typescript and the presence of a tsconfig.json is a reasonable heuristic.

It should fallback to csf format if csf-ts is unavailable for the given project type, since we can't guarantee that new project types will always have a csf-ts template.

@Keraito
Copy link
Contributor

Keraito commented Dec 19, 2018

I went through the quick start guides of all relevant frameworks/setups:

Framework/Setup Indications of TypeScript
CRA2 w/TS typescript devDependency, tsconfig.json optional. The React starter on the TS documentation site uses a version of CRA that lacked TS support.
Angular tsconfig.json, tslint.json, typescript devDependency
Babel tsconfig.json, typescript devDependency
React Native tsconfig.json, typescript devDependency
Vue tsconfig.json, typescript devDependency
Knockout tsconfig.json, typescript devDependency
JS -> TS tsconfig.json, typescript devDependency

The most straightforward way to detect a TS project is to check devDependencies on typescript. If we want to cover cases where the previous condition might not be inclusive enough (global install of typescript for example), we could also check for the existence of tsconfig.json.

@stale stale bot added the inactive label Jan 10, 2019
@Keraito Keraito self-assigned this Jan 24, 2019
@stale stale bot removed the inactive label Jan 24, 2019
@Keraito
Copy link
Contributor

Keraito commented Feb 10, 2019

I looked into this: currently, we're using merge-dirs to copy the generator template files (which are js files) over to the user's directory. We can either create new TypeScript templates for every generator (not preferable), or just change the extensions.

  • We can either change (rename the files) the extension before the merging is done. This would mean that we need to duplicate + apply those changes at the user's local side. I'm not sure how well this goes with usages via npx f.e. and personally I don't like this approach.
  • We can try to do it after, but the structure of the generator templates are all different, so it would require a lot of effort to distinguish between Storybook specific files or existing user's files.
  • Or we can do it during the merging, which is prefered. But currently this while process is managed by merge-dirs. I've opened a PR (Add file modification before merging binocarlos/merge-dirs#8) that allows us to apply changes onto files during merge, which should make generating TypeScript files way easier. The repo doesn't seem to be quite maintained anymore, so we will have to see if any progress will be made in the PR.

@stale stale bot added the inactive label Mar 3, 2019
@shilman
Copy link
Member Author

shilman commented Mar 3, 2019

@Keraito We could also switch to using recursive-copy which already supports this feature:

https://www.npmjs.com/package/recursive-copy

@stale stale bot removed the inactive label Mar 3, 2019
@Keraito
Copy link
Contributor

Keraito commented Mar 12, 2019

Yeah that sounds good @shilman, since the PR I created at merge-dirs still hasn't received any attention.

@Keraito
Copy link
Contributor

Keraito commented Mar 27, 2019 via email

@shilman
Copy link
Member Author

shilman commented Jun 20, 2019

@Keraito #7100

@storybookjs storybookjs deleted a comment from stale bot Jun 20, 2019
@storybookjs storybookjs deleted a comment from stale bot Jun 20, 2019
@storybookjs storybookjs deleted a comment from stale bot Jun 20, 2019
@storybookjs storybookjs deleted a comment from stale bot Jun 20, 2019
@storybookjs storybookjs deleted a comment from stale bot Jun 20, 2019
@storybookjs storybookjs deleted a comment from stale bot Jun 20, 2019
@stale stale bot added the inactive label Jul 11, 2019
@storybookjs storybookjs deleted a comment from stale bot Jul 12, 2019
@stale stale bot removed the inactive label Jul 12, 2019
@stale stale bot added the inactive label Aug 2, 2019
@storybookjs storybookjs deleted a comment from stale bot Aug 2, 2019
@stale stale bot removed the inactive label Aug 2, 2019
@shilman shilman added the todo label Aug 2, 2019
@yannbf yannbf self-assigned this Apr 21, 2020
@shilman
Copy link
Member Author

shilman commented Apr 27, 2020

Yowza!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.0-alpha.44 containing PR #10547 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants