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

CLI: Update renderer templates to provide correct typescript examples #21647

Merged
merged 2 commits into from
Mar 20, 2023

Conversation

yannbf
Copy link
Member

@yannbf yannbf commented Mar 17, 2023

Closes #21179

What I did

There was a change in the naming of directories inside of the CLI where template/cli/ts was split into template/cli/ts-3-8 and template/cli-ts-4-9, but not all renderers had the directories renamed. This does that for:

  • html
  • web-components
  • svelte

There's also Preact, which doesn't have typescript examples at all, but given it involves a little extra work (there's some conflicts with React errors), I'd say it could be done in a separate PR.

How to test

  1. Run a sandbox for template, e.g. yarn task --task sandbox --start-from auto --template html-vite/default-ts
  2. Check that the generated stories are typescript

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

@yannbf
Copy link
Member Author

yannbf commented Mar 17, 2023

@kasperpeulen @ndelangen given that we now provide a way to get templates from a community package, I wonder if the ts directory name change impacted them? Maybe we should also try and fallback to cli/template/ts?

@kasperpeulen kasperpeulen requested a review from IanVS March 17, 2023 11:50
@kasperpeulen
Copy link
Contributor

Tagging @IanVS as he was involved in changing the template names

@IanVS
Copy link
Member

IanVS commented Mar 17, 2023

Ah, yeah my PR started out to only handle ts-legacy templates, and it evolved from there so I guess I missed all the normal ts folders in renderers that didn't have ts-legacy.

we now provide a way to get templates from a community package

Can you point me to how that's documented? Do we know of any packages doing this so far?

Copy link
Member

@IanVS IanVS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just wondering this morning why web-components didn't have TS in the sandbox. Thanks for cleaning up after me here.

@yannbf yannbf force-pushed the fix/provide-correct-ts-stories branch from 808602d to 52a318b Compare March 20, 2023 11:00
Copy link
Member

@IanVS IanVS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should be clear for anyone creating a community template that can be installed via the CLI what the correct template directory names should be, but I think it's also good to allow a fallback if they don't completely follow it. @jonniebigodes do we have documentation for the community building new frameworks to install via our CL? I'm not sure where I'd even look for that.

@yannbf
Copy link
Member Author

yannbf commented Mar 20, 2023

I think we should be clear for anyone creating a community template that can be installed via the CLI what the correct template directory names should be, but I think it's also good to allow a fallback if they don't completely follow it. @jonniebigodes do we have documentation for the community building new frameworks to install via our CL? I'm not sure where I'd even look for that.

I agree. We do have a fallback mechanism, but honestly I think there's no documentation anywhere 😓

@yannbf yannbf merged commit bbdb548 into next Mar 20, 2023
@yannbf yannbf deleted the fix/provide-correct-ts-stories branch March 20, 2023 13:50
@jonniebigodes
Copy link
Contributor

@yannbf and @IanVS, we have the get started guide on building a framework here, but we don't have anything related to this. We'll need to expand that one and provide a more technical approach to how to build a framework and feature also the CLI templates as a part of it, mostly because it's where users will find out how to write stories for their components using a framework.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: HTML and Lit CLI templates for Typescript generate Javascript stories instead
4 participants