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

[feat]: add custom-elements-manifest analyzer #48

Closed
bennypowers opened this issue Aug 17, 2021 · 4 comments · Fixed by #50
Closed

[feat]: add custom-elements-manifest analyzer #48

bennypowers opened this issue Aug 17, 2021 · 4 comments · Fixed by #50
Assignees

Comments

@bennypowers
Copy link
Member

bennypowers commented Aug 17, 2021

Steps:

  1. add @custom-elements-manifest/analyzer as devdep to all templates
  2. add analyze script to all templates
  3. profit

cc @thepassle

@thepassle
Copy link
Member

thepassle commented Aug 17, 2021

Some things to think about are:

  • Do we want to add the CEM to the users .gitignore, and then make sure the analyzer runs on prepublish?

I know some people have strong opinions on code reviews/making sure that the CEM has updated correctly, I also know people wont like seeing CEM changes in their PRs necessarily, so we'd have to make some kind of decision there.

  • When do we run CEM?

Clever cloud starts the analyzer in watch mode kind of as a side effect of the web dev server config, so the analyzer will always run when the server runs. We could also consider creating a separate dev-server plugin to run the analyzer, maybe thats a bit 'cleaner'.

It would be nice to have the analyzer run in watch mode especially when the new version of storybook releases and makes its way into storybook-prebuilt, because then users will have up-to-date knobs/controls as they write code.

@bennypowers
Copy link
Member Author

i think we shouldn't check the manifest in. imo it's a build artifact
for the dev server plugin, I think we should develop that separately, then add it here as a feat.

should that plugin live in cem repo or dev server repo cc @LarsDenBakker
we have infra to add a plugin to cem repo, seems natural to me to put it there

@thepassle
Copy link
Member

i think we shouldn't check the manifest in. imo it's a build artifact

I agree. Maybe we go with that for now, we can always change it

should that plugin live in cem repo or dev server repo cc @LarsDenBakker

I think its fine to add it to the cem repo

@bennypowers
Copy link
Member Author

ok i'll start here then swing back to the build plugin if i have time

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

Successfully merging a pull request may close this issue.

2 participants