Skip to content

feat: Community adders #24

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

Merged
merged 17 commits into from
Sep 15, 2024
Merged

feat: Community adders #24

merged 17 commits into from
Sep 15, 2024

Conversation

AdrianGonz97
Copy link
Member

@AdrianGonz97 AdrianGonz97 commented Sep 13, 2024

This PR adds support for community adders.

Usage

There are 3 ways to execute a community adder:

"Officially unofficial" community adders

The first is with our maintained and versioned list of "officially unofficial" community adders (should we come up with a name for these?). These are adders that we've vetted and keep a special reference to in the repo. Here's what they may look like in TS:

// `community/unplugin-icons.ts`
import type { CommunityAdder } from '../packages/adders/_config/community';

export default {
	name: 'UnoCSS',
	description: 'The instant on-demand Atomic CSS engine',
	category: 'CSS',
	npm: '[email protected]',
	repo: 'https://github.com/owner-name/repo-name',
	website: 'https://unocss.dev',
	logo: 'unocss.svg'
} satisfies CommunityAdder;

with these references, users can specify them simply:

sv add --community unocss unplugin-icons supabase

and if they misspell something, they'll get a helpful list of available options:

Invalid community adder specified: 'foo'
Available options: unocss, unplugin-icons, supabase, ...

Community adder directives

The other 2 variations can be used to specify almost any arbitrary NPM package as their adder via directives (as long as the packages has @svelte-cli/core as a dependency).

The npm: directive can be used to specify any NPM package:

sv add --community npm:package-name
# specifying versions works too
sv add --community npm:[email protected]

This is useful for adders that haven't been added to our "officially unofficial" registry yet.

The file: directive can be used to specify any local package:

sv add --community file:./path/to/package

This one is especially useful when you're creating a community adder locally and you want to test it.

Limitations

Since we're manually downloading these packages, the one limitation is that we won't resolve any dependencies for these adders. That is to say that external dependencies outside of @svelte-cli/core will not be allowed. sv will throw a helpful error when this occurs:

Invalid adder package detected: 'npm:foo-bar'
Community adders should not have any external 'dependencies' besides '@svelte-cli/core'. Consider bundling your dependencies if they are necessary

If they really need a dependency, they can bundle them as part of their adder package.

Additional notes

  • community/unocss.ts and community/unplugin-icon.ts are just sample files for now. They don't reference any real NPM packages yet
  • you can test out the npm: directive using --community npm:@cokakoala/adder-template
  • the adder-template repo lives here atm, but we plan on moving it directly into this repo (we can probably place at the root: templates/adder-template-js)

Copy link

pkg-pr-new bot commented Sep 13, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/sveltejs/cli/@svelte-cli/ast-manipulation@24
pnpm add https://pkg.pr.new/sveltejs/cli/@svelte-cli/ast-tooling@24
pnpm add https://pkg.pr.new/sveltejs/cli/sv@24
pnpm add https://pkg.pr.new/sveltejs/cli/@svelte-cli/core@24

commit: 31925c8

Copy link
Member

@manuel3108 manuel3108 left a comment

Choose a reason for hiding this comment

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

Code wise everything looks good. I'm still on my tablet, so there is no way to test at this point.

I think we should also add new merge check, that validates community adder properties. Eseiest thing would be to check if the reps of the package as we are doing in another place and to check if the categorie exists. We could also add a check if the package exists on npm, the gh repo exists etc.

But besides that I don't have anything to add

@benmccann
Copy link
Member

I wonder if PRs to the community directory will create too much noise and we'd prefer them to be in a separate repo. I also wonder if we'd prefer to dynamically fetch the JSON rather than bundling it at build-time. E.g. we might want to display a list of community adders on the website as well. But I suppose those are things that are easy enough to change in the future if we'd like and so I think we can just go with anything that works for the moment rather than overthinking it.

The one thing that I do think would be nice to have somewhat upfront is a way to see what community adders are available. Maybe a new command like sv list or sv search or something like that would be a good way to do it?

@AdrianGonz97
Copy link
Member Author

I wonder if PRs to the community directory will create too much noise and we'd prefer them to be in a separate repo. I also wonder if we'd prefer to dynamically fetch the JSON rather than bundling it at build-time. E.g. we might want to display a list of community adders on the website as well.

That's a fair point. Nuxt modules also uses an api to fetch their metadata for community modules, which is viable option.

Although, the idea behind having them bundled is for versioning. If we were to fetch from an API, it'll always return with the latest versions of the community adders (unless we version the API too). If they're bundled in, then when users run [email protected] they can expect to have the same outcomes every time they execute it.

The one thing that I do think would be nice to have somewhat upfront is a way to see what community adders are available. Maybe a new command like sv list or sv search or something like that would be a good way to do it?

this is a neat idea and something to consider. it might be best to have that be in its own PR

@manuel3108
Copy link
Member

I wonder if PRs to the community directory will create too much noise and we'd prefer them to be in a separate repo

As suggested in my last comment I think we should be able to automate a lot of the checks, but we could easily do this in a followup pr. It should basically only require an approval and merge from us.

The one thing that I do think would be nice to have somewhat upfront is a way to see what community adders are available. Maybe a new command like sv list or sv search or something like that would be a good way to do it?

I would expect sv add --community to do exactly this

@benmccann
Copy link
Member

Oh I had assumed it would be getting the latest version of each adder. I think it'd be super noisy if community adder authors had to update the version number here each time they released a new version. Maybe what we could do is have the community adder specify a peer dependency on sv and then print a warning that we can't run the community adder

@AdrianGonz97
Copy link
Member Author

AdrianGonz97 commented Sep 14, 2024

Oh I had assumed it would be getting the latest version of each adder.

Doh, that's because I forgot to include the tag in what community adder metadata would look like (though i got it right on discord 😅). It should've been:

Perhaps that's enough justification to have it's own version field, huh?

I think it'd be super noisy if community adder authors had to update the version number here each time they released a new version.

yea, I get that. We would have to publish a new release for sv everytime a new update occurs. Maybe these updates could be a weekly/biweekly thing? e.g. every Sunday/other Sunday, have sv cut a new release if there are any pending updates for community adders?

I just feel uneasy on pulling the latest changes for community adder versions that we haven't vetted yet.

Maybe what we could do is have the community adder specify a peer dependency on sv and then print a warning that we can't run the community adder

uhh, im not quite sure how a community adder's peer dep on sv would help in this case?

@benmccann
Copy link
Member

I just feel uneasy on pulling the latest changes for community adder versions that we haven't vetted yet.

It seems impossible to me that we would be able to vet the code for every version of every adder. We've not been able to keep up with adding packages to https://www.sveltesociety.dev/packages and there's no review of the code that's done there. https://nuxt.com/modules has over 240 packages. That's thousands of reviews that would have to have been done. I think if we wanted to vet them we'd have to have some sort of automated way like only allow the authors to use imports from a whitelist. I've been assuming we're not going to do any real investigation and that's why the warning/caveat is so important.

uhh, im not quite sure how a community adder's peer dep on sv would help in this case?

I thought the concern was that we would make a major release of the core library and break adders

@manuel3108
Copy link
Member

I think a custom version field makes sense yes, especially that people don't forget it, if we want to pin versions. I don't think we should, as the cli makes it clear that those are handled by the community. It would create much maintanace overhead.
If people want to use a specific version they still can, assp the pr description suggests.

Assuming we still use changes (which we still need to setup) we are infull control other releases, so I don't see a problem here.

@AdrianGonz97 AdrianGonz97 merged commit 0edd8b3 into main Sep 15, 2024
5 checks passed
@AdrianGonz97 AdrianGonz97 deleted the feat/community-adders branch September 15, 2024 21:05
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 this pull request may close these issues.

3 participants