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

Integrate CLI? #1360

Closed
Rich-Harris opened this issue Apr 21, 2018 · 4 comments · Fixed by #1389
Closed

Integrate CLI? #1360

Rich-Harris opened this issue Apr 21, 2018 · 4 comments · Fixed by #1389

Comments

@Rich-Harris
Copy link
Member

Raised this in Gitter, but it deserves an issue: should we move svelte-cli into this repo?

We often tell people not to use the CLI, for good reason, but the reality is that it does make sense for some people, and it could be the easiest way to try Svelte out:

echo "<h1>Hello {name}</h1>" > Test.html
npx svelte compile Test.html

That doesn't work at the moment — you'd have to do this, which is much less elegant:

echo "<h1>Hello {name}</h1>" > Test.html
npx svelte --package svelte-cli compile Test.html

We can't not have a CLI, so we should try and at least make it good. Keeping it here would ensure that it was better maintained. And it doesn't add much overhead to the package. I know that the trend for CLIs has generally been in the other direction, but I don't think there's really a good reason for it other than some misplaced sentiment that modularity is more important than user experience.

Thoughts?

@Conduitry
Copy link
Member

Eehhhhhhh having it in the same package as the compiler does give me some pretty bad vibes. What if svelte-cli specifically did not npm-depend on svelte, and would instead look for and require a local installation of svelte? Yeah that's an extra step, but what exactly are we trying to eliminate here? This would stop the problems of it being really unclear what version of svelte the cli is using and how one should go about updating that.

@Rich-Harris
Copy link
Member Author

What if svelte-cli specifically did not npm-depend on svelte, and would instead look for and require a local installation of svelte?

You mean a relative require based on cwd? (since a globally installed svelte-cli would fail otherwise.) Could do, though it's a pretty awful/confusing experience for anyone using npx, and doesn't address the fact that npx svelte --package svelte-cli is a bit gross.

what exactly are we trying to eliminate here?

The overhead of a) for us, maintaining the packages separately, and b) for users, having to worry about the relationship between the two packages, thinking about global versus local installations, and dealing with headaches arising from version incompatibilities, as happened today.

For context, the Rollup API and CLI exist in the same package, and it's never crossed our minds to separate them into two separate things.

@Conduitry
Copy link
Member

All right, yeah that's fair. I did just take a peek at how big the svelte-cli dist is on npm and it's smaller than I was thinking it'd be. Probably I was being influenced by other CLIs I've used that pull in mammoth command line parsers with a zillion non-bundled dependencies.

This does seem like a reasonable way to simplify things. I was just trying to think how, technically, this ought to be implemented. Probably simplest would be to have separate entry points that produce separate standalone .js files - but it would be nice to not duplicate all the compiler code in the cli .js. This a job for the external: option in rollup.config.js? I'm not as familiar with that as perhaps I should be.

@Rich-Harris
Copy link
Member Author

One approach is to use code-splitting - that's what Sapper does. In this case, I think it probably would be simpler to use external as you suggest, and use the paths option to rewrite the require statement to point at the compiler.

Rich-Harris added a commit that referenced this issue Apr 30, 2018
Rich-Harris added a commit that referenced this issue Apr 30, 2018
Rich-Harris added a commit that referenced this issue Apr 30, 2018
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