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

Add package command #2882

Merged
merged 3 commits into from
Nov 23, 2021
Merged

Add package command #2882

merged 3 commits into from
Nov 23, 2021

Conversation

Rich-Harris
Copy link
Member

If component libraries are to be a first-class use case for Kit, it stands to reason that npm run package should be available in new projects alongside npm run build.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@changeset-bot
Copy link

changeset-bot bot commented Nov 23, 2021

🦋 Changeset detected

Latest commit: 2b70f1e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
create-svelte Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@benmccann
Copy link
Member

Or should it be part of svelte-create? An npm run package command wouldn't make as much sense if you're building a web app instead of a library. I wonder if it would end up confusing people because they think they're packaging their app for production deployment or something?

@Conduitry
Copy link
Member

I agree about worrying that this might be confusing for people who are just writing an app and not a reusable library.

I'm not sure what @benmccann's suggesting though. That this become one of the prompts in the create-svelte CLI? Something else?

@dummdidumm
Copy link
Member

Also in favor of a prompt "do you want to create a library or an app?"

@Rich-Harris
Copy link
Member Author

I think npm run build is a widely-enough used convention that if someone opts to npm run package instead it's a definite case of PEBKAC. The docs (https://kit.svelte.dev/docs#command-line-interface) are fairly clear in any case.

Disagree that a prompt is appropriate — the whole point of this feature is that apps and libraries are the same thing (an app has an internal library, a library has a demo/docs app, the only difference is which bit is user-facing). If the only thing the prompt changes is whether or not "package": "svelte-kit package" gets added to the package.json, that's a pretty meagre payoff for adding another (potentially confusing) decision at project init time.

I think the solution here is something I've been meaning to get round to anyway — add some language when svelte-kit package completes along the following lines:

Successfully built library: src/lib -> package
To publish your package:
$ cd package
$ npm publish

@Conduitry
Copy link
Member

👍 Adding that message when the package command completes would address my concerns in a tidy enough way, and does seem better than adding another prompt to the CLI.

@Rich-Harris
Copy link
Member Author

Alright, added that. I really think we need to revisit this —

Cannot generate a "svelte" entry point because the "." entry in "exports" is missing. Please specify one or set a "svelte" entry point yourself

— it's very confusing and shouldn't be necessary. Will open a separate issue though as it will probably mean coordinating with the plugins

@Rich-Harris Rich-Harris merged commit cc45b2c into master Nov 23, 2021
@Rich-Harris Rich-Harris deleted the add-package-command branch November 23, 2021 23:03
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