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

chore: add basic svelte-add stuff #2

Merged
merged 18 commits into from
Aug 20, 2024
Merged

Conversation

manuel3108
Copy link
Member

@manuel3108 manuel3108 commented Aug 16, 2024

The goal here is to get the most basic integration going and to avoid typing / linting errors. Nothing fancy.

You can actually already test the most basic integration. Checkout the repo and run the build command (!IMPORTANT!, for create-svelte template stuff creation) and start the dev server after that if you like. To run the cli use:

node .\packages\cli\dist\bin.js

Todos:

  • fix linting errors
  • convert create-svelte to TS, so that we can properly integrate it in our build setup
  • Think about a solution for avoiding the need to run a separate command for generating the create-svelte templates.
  • Check for typing errors with pnpm check

Notable changes

  • started converting cli / create-svelte to TS, so that it can make proper use of or other packages, without the need of bundling the other packages beforehand
  • changed templates output directory for cli from dist to dist-templates, so that we can keep using dist for our bundling output, to keep it unified across packages.
  • tests from svelte-add can be handled in another pr later on.

@manuel3108 manuel3108 force-pushed the setup-svelte-add-basics branch 2 times, most recently from d6030ed to b981068 Compare August 16, 2024 15:49
@dominikg
Copy link
Member

other packages in the svelte org are developed with unbundled js, jsdoc and dts-buddy for types. should it be the same here?

how is the integration between different parts going to work? l suppose we are going to look into moving create, migrate, package from the kit repo over here or import these packages to make them available through sv later on?

@benmccann
Copy link
Member

other packages in the svelte org are developed with unbundled js, jsdoc and dts-buddy for types. should it be the same here?

I don't think it's necessarily required here. A lot of the reason for doing that is because the other packages are libraries intended to be used in other projects. That's mostly not how this package is used though

how is the integration between different parts going to work? l suppose we are going to look into moving create, migrate, package from the kit repo over here or import these packages to make them available through sv later on?

create has already been copied here. I'll manually sync over any changes made in the kit repo to this one until this CLI is announced at Svelte Summit in October and then we can delete it from the kit repo. I'm not sure about migrate and package. Those would require more discussion.

@manuel3108
Copy link
Member Author

other packages in the svelte org are developed with unbundled js, jsdoc and dts-buddy for types. should it be the same here?

I think that's a good point that needs to be discussed. Back in January or so svelte-add was written in js with jsdoc. Since there are a few places where we use types quite heavily (i.e. ast-manipulation) it just did not feel right and was adding a lot of comments just for type information.

how is the integration between different parts going to work? l suppose we are going to look into moving create, migrate, package from the kit repo over here or import these packages to make them available through sv later on?

create has already been copied here. I'll manually sync over any changes made in the kit repo to this one until this CLI is announced at Svelte Summit in October and then we can delete it from the kit repo. I'm not sure about migrate and package. Those would require more discussion.

I think in the long run this definitely makes sense. Due to the timeline, I don't think it matters if we do it before or after the summit.

packages/cli/package.json Outdated Show resolved Hide resolved
packages/clack-prompts/package.json Outdated Show resolved Hide resolved
@dominikg
Copy link
Member

create has already been copied here. I'll manually sync over any changes made in the kit repo to this one until this CLI is announced at Svelte Summit in October and then we can delete it from the kit repo.

i think this needs to be discussed and checked for ergonomics of development for kit related packages/commands. should they even live in this repo or continue to be in kit?

keeping a synced copy hasn't worked well back when vite-plugin-svelte started and moving create-svelte out of the kit repo is going to make testing changes in kit with it much harder for example.

a modular approach to sv where it loads extra packages for commands can ensure we keep the modules close to their focus. some like add may live here where others may live in kit or language-tools or even in third party repos if we add extension points/plugins to sv for that.

did you check how others like nuxi or ng handle their deps?

maybe i should break this down into issues/discussions? but feels important to me to get bootstrapped in the right direction. this is going to have major impact on the dx for us and contributors and even the ux for the users.

@benmccann
Copy link
Member

It's possible that we could move this code back into the kit repo longer-time. For now we want it in a private repo though so that we can develop without prying eyes and do a release announcement at Svelte Summit

keeping a synced copy hasn't worked well

it's only a very temporary thing until this package is announced. i'm not worried about being able to do it for the next two months. we definitely won't have two copies of the create-svelte templates long-term

moving create-svelte out of the kit repo is going to make testing changes in kit with it much harder for example

I can't remember having a change that really touched kit and create-svelte at the same time in a major way

a modular approach to sv where it loads extra packages for commands can ensure we keep the modules close to their focus

One thing we do need to figure out is whether we want to try to load community adders in some fashion or how that would work. For the core stuff though, I hate developing across multiple repos. It's such a pain. It's really nice to have this stuff live in the same repo. create-svelte is far more closely tied to the sv CLI than it is to SvelteKit

did you check how others like nuxi or ng handle their deps?

no. I'm definitely curious though if anyone's got time to investigate

@AdrianGonz97
Copy link
Member

AdrianGonz97 commented Aug 16, 2024

a modular approach to sv where it loads extra packages for commands

A modular approach could be interesting. It's been something that I've had in the back of my mind for a little now.

Each command would have their own discrete package that gets downloaded and executed (e.g. sv add ... could call npx @svelte-cli/add ... under the hood) on usage. If we do this, the sv package will be super light since it's only job would be to act as a centralized distributor for each command, ensuring that only the necessary packages will ever be downloaded, which is compelling. One thing that does worry me with that approach would be the perceived responsiveness of sv.

imagine running npx sv migrate svelte-4 on a slow connection, downloading its 36MB of dependencies and modules. npx sv would be super quick, but the following migrate svelte-4 will feel significantly slower. From the user's perspective, it may even seem confusing as to why it's taking ages to run a single command when it looks like they've "already downloaded" the sv package. It may be a worthy tradeoff though.

if we go down that route, we could structure the packages like so:

.
├── bin (entrypoint for sv)
│   └── index.js
├── packages
│   ├── add
│   ├── create (or init)
│   └── migrate
└── package.json

moving create-svelte out of the kit repo is going to make testing changes in kit with it much harder for example.

would it? i don't think I see it being used directly anywhere in the repo for testing, unless you meant something else?

i'm not particularly convinced that having the packages be located all over the place is the best idea either. i distinctly remember being confused the first time i wanted to check out the source for svelte-migrate and was surprised to find that create-svelte and svelte-migrate lived in the kit repo

maybe i should break this down into issues/discussions? but feels important to me to get bootstrapped in the right direction.

that would be great!

@manuel3108 manuel3108 force-pushed the setup-svelte-add-basics branch from ffec610 to 491d0ae Compare August 17, 2024 06:40
@manuel3108 manuel3108 changed the title DRAFT: chore: add basic svelte-add stuff chore: add basic svelte-add stuff Aug 17, 2024
rollup.config.js Outdated Show resolved Hide resolved
@dominikg
Copy link
Member

would it? i don't think I see it being used directly anywhere in the repo for testing, unless you meant something else?

it is used if you run pnpm test in packages/create-svelte, it runs the local create-svelte to generate projects in .test-tmp and execute various package.json scripts like format,check and build on them to ensure that the output of create-svelte always ends up creating a working setup.

The name create-svelte can be a bit misleading, it really is create-svelte-kit. Both changes to kit itself or create-svelte can lead to the output of create-svelte no longer working and building a test similar to the one we have now across multiple repos is harder. (not impossible, we can look into using svelte-ecosystem-ci for it)
The feedback when changing create-svelte or svelte-kit is going to be less direct though.
Due to the tight relation between sveltekit and create-svelte in might actually make more sense to keep create-svelte in the kit repo and import it here, OR have the kit repo import sv and use that with templates still hosted in the kit repo.

prior to create-svelte, the original svelte+rollup template was loaded with degit and in the past i did use degit in svite to load templates from github at execution time rather than shipping them with the cli.
Both approaches have pros and cons, ultimately we need to be able to ship sv so that users can always create apps with the latest templates.

svelte-package also is using sveltekit under the hood and the same issues around testing apply, it cannot be used without kit so i'd say it makes sense to host it in the kit monorepo.

svelte-migrate is a different story as it has migrations for both svelte and sveltekit.

I really like the idea of sv being more of a shell that does not change on its own a lot, but rather pulls in current data on install/execute. This raises the question if you want to download it on every invocation or if it would be a devDep too and keep things stored/updated locally in node_modules/.cache/sv ?

@benmccann
Copy link
Member

The feedback when changing create-svelte or svelte-kit is going to be less direct though.
Due to the tight relation between sveltekit and create-svelte in might actually make more sense to keep create-svelte in the kit repo and import it here, OR have the kit repo import sv and use that with templates still hosted in the kit repo.

I think the relation is tighter between create-svelte and svelte-add than it is between create-svelte and kit. The main time create-svelte needs to be updated in response to kit changes is when there's a new SvelteKit major. That's relatively rare and can be done on a slight delay in the repo. On the otherhand, changing the templates has a somewhat high probability of breaking the adders as the problem they're trying to solve is pretty difficult to do robustly as much as we try.

svelte-package also is using sveltekit under the hood and the same issues around testing apply, it cannot be used without kit so i'd say it makes sense to host it in the kit monorepo.
svelte-migrate is a different story as it has migrations for both svelte and sveltekit.

Yeah, I'm less familiar with those, but they could make sense to continue living in the SvelteKit repo. I think for now we'll not try to include them because they would pull in a lot of new dependencies, but I have been working to slim them down and maybe we can revisit after the initial release.

I really like the idea of sv being more of a shell that does not change on its own a lot, but rather pulls in current data on install/execute. This raises the question if you want to download it on every invocation or if it would be a devDep too and keep things stored/updated locally in node_modules/.cache/sv ?

It could be a bit of a mix as well. We could have some things live in this monorepo and some things pulled in. I don't think we need to decide it all right now though

@benmccann benmccann merged commit 8926e6b into main Aug 20, 2024
2 checks passed
@benmccann benmccann deleted the setup-svelte-add-basics branch August 20, 2024 03:22
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.

4 participants