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.

Sign up for GitHub

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

Jump to bottom

[fix] add svelte field when packaging #2431

Merged
merged 5 commits into from
Sep 16, 2021
Merged

[fix] add svelte field when packaging #2431

merged 5 commits into from
Sep 16, 2021

Conversation

dummdidumm
Copy link
Member

@dummdidumm dummdidumm commented Sep 15, 2021 •
edited
Loading

Several heuristics in Kit/vite-plugin-svelte to tell Vite to mark Svelte packages rely on the "svelte" property. Vite/Rollup/Webpack plugin can all deal with it.

Fixes #1959

@dominikg @bluwy I didn't quite follow through your discussion over at vite-plugin-svelte, but I think this change is needed for Svelte packages to properly get detected, right?

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

Simon Holthausen added 2 commits September 15, 2021 09:54
[fix] add svelte field when packaging
1208ed6 
Several heuristics in Kit/vite-plugin-svelte to tell Vite to mark Svelte packages rely on the "svelte" property. Vite/Rollup/Webpack plugin can all deal with it.

Fixes #1959
changeset
5079fb4
@changeset-bot
Copy link

changeset-bot bot commented Sep 15, 2021 •
edited
Loading

🦋 Changeset detected

Latest commit: afc7985

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

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit 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

@dominikg
Copy link
Member

yes, thats right. the svelte field is relied upon. We should also make sure that exports contains
"./package.json":"./package.json", this is important for both rollup-plugin-svelte and vite-plugin-svelte to be able to read the svelte field.

@dummdidumm
Copy link
Member Author

yes, thats right. the svelte field is relied upon. We should also make sure that exports contains
"./package.json":"./package.json", this is important for both rollup-plugin-svelte and vite-plugin-svelte to be able to read the svelte field.

This is already the case 👍

@dominikg
Copy link
Member

dominikg commented Sep 15, 2021 •
edited
Loading

actually the "svelte" field is only needed for packages that export ".svelte" components. Utilities that come without any components, eg just some fancy store or context utility can still ship without a "svelte" field.
So maybe a warning to add it would be better, or somehow find out if any .svelte is exported?

Or is it implied that package is always used for ui, as utilities generally would not need to go through preprocess ?

@bluwy
Copy link
Member

bluwy commented Sep 15, 2021

Yeah @dominikg's comment are spot on. The svelte field is used to workaround Vite's prebundling process, and is only needed if they export Svelte components. I don't think Kit reads the svelte field anymore though, the logic has been moved to vite-plugin-svelte.

@dummdidumm @bluwy
Update packages/kit/src/packaging/index.js
633e8c2 
Co-authored-by: Bjorn Lu <34116392+bluwy@users.noreply.github.com>
bluwy
bluwy approved these changes Sep 16, 2021
View reviewed changes
Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

LGTM. I think we should apply @ignatiusmb's suggestion too so we have two different warnings for the two cases.

@dummdidumm dummdidumm merged commit 3067422 into master Sep 16, 2021
@dummdidumm dummdidumm deleted the add-svelte-export branch September 16, 2021 12:55
@github-actions github-actions bot mentioned this pull request Sep 16, 2021
Merged
sidharthv96 added a commit to sidharthv96/kit that referenced this pull request Sep 19, 2021
@sidharthv96
Merge branch 'master' of https://github.com/sveltejs/kit
d5059c5 
* 'master' of https://github.com/sveltejs/kit: (322 commits)
  Version Packages (next) (sveltejs#2438)
  [fix] revert sveltejs#2354 and fix double character decoding a different way (sveltejs#2435)
  chore: update dependencies (sveltejs#2447)
  [docs] add link to envPrefix option in env var FAQ (sveltejs#2445)
  Fix invalid changeset. Thanks GitHub bot :-p
  [feat] use the Vite server options in dev mode (sveltejs#2232)
  [fix] provide valid value for `letter-spacing` CSS property in template (sveltejs#2437)
  [docs] fix typo (sveltejs#2443)
  [fix] add svelte field when packaging (sveltejs#2431)
  Version Packages (next) (sveltejs#2428)
  [chore] update lockfile
  [fix] update to TS 4.4.3 (sveltejs#2432)
  [chore] add links to repository and homepage to package.json (sveltejs#2425)
  docs: use fragment for prefetching link (sveltejs#2429)
  [fix] encodeURI during prerender (sveltejs#2427)
  Version Packages (next) (sveltejs#2420)
  revert change to versioning during release workflow
  chore: update vite-plugin-svelte (sveltejs#2423)
  [chore] expose Body generic to hook functions (sveltejs#2413)
  [feat] adapter-node entryPoint option (sveltejs#2414)
  ...
@Rich-Harris Rich-Harris mentioned this pull request Nov 23, 2021
Open
@techniq techniq mentioned this pull request Dec 15, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ignatiusmb ignatiusmb ignatiusmb left review comments

@bluwy bluwy bluwy approved these changes

@dominikg dominikg dominikg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add svelte exports field to package json when running package command
4 participants
@dummdidumm @dominikg @bluwy @ignatiusmb