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

breaking: update svelte-package #8922

Merged
merged 44 commits into from
Feb 16, 2023
Merged

breaking: update svelte-package #8922

merged 44 commits into from
Feb 16, 2023

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Feb 7, 2023

Migration instructions

Most of the things explained below are migrated automatically using npx svelte-migrate package

No more generated package.json

@sveltejs/package version 2 no longer generates a package.json inside the output directory. Instead, you should define the exports manually, as you know best what's the entry points to your library are.

The migration script will help you by runing the same logic the previous major version @sveltejs/package ran and output an exports map into your root package.json. It might look something like this:

{
  "name": "your-package-name".
   ..,
+ "exports": {
+     ".": "./package/index.js",
+     "./package.json": "./package.json",
+     "./SomeComponent.svelte": "./package/SomeComponent.svelte"
+  }
}

We encourage you to review the exports map carefully and familiarize yourself with how it works in general (more on that in the documentation). You also might want to clean up the exports map and for example only provide one entry point, the root - probably a good fit for most packages. Note that this would be a breaking change, so if you do this, make sure you do that in a major version release.

If you want to keep copying your package.json over, you can do this through a simple custom script that you would invoke after svelte-package:

import fs from 'fs';

// read file into JSON
const pkg = JSON.parse(fs.readFileSync('package.json', 'utf-8'));

// adjust pkg json however you like ..

// write it to your output directory
fs.writeFileSync(
  './package/package.json', // path to your output directory may vary
  JSON.stringify(pkg, null, 2)
);

typesVersions

You'll probably notice a new typesVersions field in your package.json. It's needed so TypeScript knows where to find deep imports like your-library/deep/import. You can read more about it in the docs.

Output directory changed to dist

Previously, the output was written to package by default. It's now written to dist by default. You can adjust this behavior through the CLI flag -o/--output. Example: svelte-package -o package.

The migration script will adjust any svelte-package invocations in your package.json's script so it's written to your current output directory (package by default). Consider adjusting your setup so the output is written to dist instead.

No more configuration options through svelte.config.js

The configuration options from svelte.config.js were removed in favor of CLI flags from the svelte-package command.

  • package.source is now -i/--input
  • package.dir is now -o/--output
  • package.emitTypes is now -t/--types
  • package.exports is removed in favor of manually authoring the exports map (also see section above)
  • package.files is removed. If you don't want some files to appear in the package, add an .npmignore file

The migration script will remove it from your svelte.config.js automatically. If you have scripts elsewhere that make use of some the options (like input or output), you need to adjust them manually.

PR description

#8825. Very WIP.

  • get tests passing
  • add package validation (ensure that pkg.exports points at files inside the output directory, and that there's a svelte condition present so that vite-plugin-svelte is able to automatically noExternal Svelte libraries
  • docs
  • migration script
  • list all the issues and PRs that will be closed by this

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 pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

@dummdidumm
Copy link
Member

dummdidumm commented Feb 7, 2023

  • What's the replacement for files? People have previously used that to for example remove test files from the output. I don't think we should have some kind of automatic import traversal as that will fall down if people want to provide some CSS or other such files that have no import anywhere and they have some instructions how to set those up differently.
  • I guess Change "svelte" path in package.json in root during dev and in package dir after npm run package #2242 and Only include lib dependencies in package #4716 are "won't fix" then?
  • "ensure that pkg.exports points at files inside the output directory..." -> this reminds me that we should make sure you can have the input directory being the output directory, in the sense that existing files are not overridden and instead it just generated d.ts files next to them
  • "... and that there's a svelte condition present" -> I think not all people need that, for example if you have a Svelte action package you only have some JS/TS files and don't need these things to run through v-p-s. I think the check should be "did I encounter any Svelte files or any SvelteKit imports". I have a WIP from some time ago lying around, which could be a start:
function create_code_analyser() {
	/** @type {Set<string>} */
	const imports = new Set();
	let uses_import_meta = false;

	/**
	 * Checks a file content for problematic imports and things like `import.meta`
	 * @param {string} content
	 */
	function analyse_code(content) {
		uses_import_meta = uses_import_meta || content.includes('import.meta.env');

		const file_imports = [
			...content.matchAll(/from\s+('|")([^"';,]+?)\1/g),
			...content.matchAll(/import\s*\(\s*('|")([^"';,]+?)\1\s*\)/g)
		];
		for (const [, , import_path] of file_imports) {
			if (import_path.startsWith('$app/')) {
				imports.add(import_path);
			}
		}
	}

	/** @param {Record<string, any>} pkg */
	function validate(pkg) {
		if (
			imports.has('$app/environment') &&
			[...imports].filter((i) => i.startsWith('$app/')).length === 1
		) {
			console.warn(
				'Avoid usage of `$app/environment` in your code, if you want to library to work for people not using SvelteKit (only regular Svelte, for example). ' +
					'Consider using packages like `esm-env` instead which provide cross-bundler-compatible environment variables.'
			);
		}

		if (uses_import_meta) {
			console.warn(
				'Avoid usage of `import.meta.env` in your code. It requires a bundler to work. ' +
					'Consider using packages like `esm-env` instead which provide cross-bundler-compatible environment variables.'
			);
		}

		if (
			!(pkg.dependencies?.['@sveltejs/kit'] || pkg.peerDependencies?.['@sveltejs/kit']) &&
			([...imports].some((i) => i.startsWith('$app/')) || imports.has('@sveltejs/kit'))
		) {
			console.warn(
				'You are using SvelteKit-specific imports in your code, but you have not declared a dependency on `@sveltejs/kit` in your `package.json`. ' +
					'Add it to your `dependencies` or `peerDependencies`.'
			);
		}

		if (
			!(pkg.dependencies?.svelte || pkg.peerDependencies?.svelte) &&
			[...imports].some((i) => i.startsWith('svelte/') || imports.has('svelte'))
		) {
			console.warn(
				'You are using Svelte-specific imports in your code, but you have not declared a dependency on `svelte` in your `package.json`. ' +
					'Add it to your `dependencies` or `peerDependencies`.'
			);
		}
	}

	return {
		analyse_code,
		validate
	};
}

@dummdidumm dummdidumm changed the title feat: update svelte-package breaking: update svelte-package Feb 7, 2023
@dummdidumm dummdidumm added breaking change pkg:svelte-package Issues related to svelte-package labels Feb 7, 2023
@Rich-Harris
Copy link
Member Author

What's the replacement for files?

.npmignore

I guess #2242 and #4716 are "won't fix" then?

Yeah. In the first case, if your code is consumable as-is then you could just point your exports at your source code. Otherwise you'd need svelte-package --watch

I think not all people need that, for example if you have a Svelte action package you only have some JS/TS files and don't need these things to run through v-p-s

I feel like it's just way simpler to always noExternal stuff — AFAICT that's the only effect adding a svelte condition would have for non-.svelte files?

@brucejo75
Copy link

@dummdidumm, @sureshjoshi, I will enter it in.

@sureshjoshi
Copy link

Only the 10 icons. Tree shaking is smart enough to know which of the imports on that file are actually used, and removes the unused ones

This doesn't work for me (unrelated, don't want to drag the conversation elsewhere). I think there are about 5 tickets across multiple repos about this - but there is production tree shaking to reduce the number of icons. Works great.

Recently, Vite started pre-bundling - so we get the benefit of not needing to download 1000 icons every time we start the dev server. However, this is still an issue in vitest.

Anyways, I have a ticket or a discord about this coming up, just wanted to capture it somewhere first :)

@ysitbon
Copy link

ysitbon commented Feb 17, 2023

@dummdidumm I'm probably wrong but now I have doubts that need to be confirmed, if I may. Suppose you have a library of 1000 icons as Svelte components. If we provide and index.js with all the icons and then use it in your application for probably 10 icons, what is included in your output 10 icons or 1000?

Only the 10 icons. Tree shaking is smart enough to know which of the imports on that file are actually used, and removes the unused ones

Thanks!

@brucejo75
Copy link

@dummdidumm , #9097

@rob-balfre
Copy link

rob-balfre commented Feb 18, 2023

Trying to upgrade svelte-select but when I run npx svelte-migrate package it errors...

$ npx svelte-migrate package
You must specify one of the following migrations: routes

...

Must of had an old migrate version somewhere... found a workaround:

$ npm i -g svelte-migrate 
$ npx svelte-migrate package

@rob-balfre
Copy link

rob-balfre commented Feb 19, 2023

Am I correct in thinking v1 generated a folder, copied over .md files and LICENCE and created a package.json? I don't remember ever setting up svelte-package to copy them over (unless I'm being stupid?!)

With v2 am I correct in saying we now use the root package.json to publish? So everything in package.json files: [...] and not excluded by .npmignore will be published to npm?

Not complaining, I'm sure its a logical update just wrapping my head around the changes. Thanks

ah ha... READ THE DOCS ROB!!

Certain files are always included, regardless of settings:

package.json
README
LICENSE / LICENCE
The file in the "main" field

renovate bot added a commit to defenseunicorns/zarf that referenced this pull request Mar 22, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@sveltejs/package](https://kit.svelte.dev)
([source](https://togithub.com/sveltejs/kit)) | [`1.0.2` ->
`2.0.2`](https://renovatebot.com/diffs/npm/@sveltejs%2fpackage/1.0.2/2.0.2)
|
[![age](https://badges.renovateapi.com/packages/npm/@sveltejs%2fpackage/2.0.2/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/@sveltejs%2fpackage/2.0.2/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/@sveltejs%2fpackage/2.0.2/compatibility-slim/1.0.2)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/@sveltejs%2fpackage/2.0.2/confidence-slim/1.0.2)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>sveltejs/kit</summary>

###
[`v2.0.2`](https://togithub.com/sveltejs/kit/blob/HEAD/packages/package/CHANGELOG.md#&#8203;202)

[Compare
Source](https://togithub.com/sveltejs/kit/compare/@sveltejs/package@2.0.1...@sveltejs/package@2.0.2)

##### Patch Changes

- fix: don't emit false positive export condition warning
([#&#8203;9109](https://togithub.com/sveltejs/kit/pull/9109))

###
[`v2.0.1`](https://togithub.com/sveltejs/kit/blob/HEAD/packages/package/CHANGELOG.md#&#8203;201)

[Compare
Source](https://togithub.com/sveltejs/kit/compare/@sveltejs/package@2.0.0...@sveltejs/package@2.0.1)

##### Patch Changes

- fix: print version when running `svelte-package --version`
([#&#8203;9078](https://togithub.com/sveltejs/kit/pull/9078))

###
[`v2.0.0`](https://togithub.com/sveltejs/kit/blob/HEAD/packages/package/CHANGELOG.md#&#8203;200)

[Compare
Source](https://togithub.com/sveltejs/kit/compare/@sveltejs/package@1.0.2...@sveltejs/package@2.0.0)

##### Major Changes

- breaking: remove `package.json` generation and package options from
`svelte.config.js`. New default output directory is `dist`. Read the
migration guide at
[sveltejs/kit#8922
to learn how to update
([#&#8203;8922](https://togithub.com/sveltejs/kit/pull/8922))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [x] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/defenseunicorns/zarf).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xNDIuMSIsInVwZGF0ZWRJblZlciI6IjM0LjE1NC43In0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@ghost
Copy link

ghost commented Mar 23, 2023

PSA: Using the files field inside the package.json in combination with a .npmignore file is weird, e.g. "Files included with the "package.json#files" field cannot be excluded through .npmignore or .gitignore." (npm docs).

We can use this pattern to remove the need for the files field in the package.json which works for us.

@kwangure
Copy link
Contributor

The files field values are glob patterns. As such, if you don't need to use an .npmignore. Use negative glob patterns.

"files": [
    "dist",
    "!**/*spec.js",
    "!**/*spec.d.ts"
]

Kit Docs are misleading since .npmignore with pkg.files doesn't Just Work™ as advertised evidenced by npm/npm#4479.

@lafkpages
Copy link

svelte-package is ignoring my .npmignore file. I have it in the root of the project, and I've tried removing package.json#files and also tried different paths for the files to ignore, like landing, dist/landing and src/lib/landing. Nothing works, it all gets packaged. Is this not the right way to ignore files when packaging?

@NatoBoram
Copy link

When making a new project, there's this key in package.json:

	"files": [
		"dist",
		"!dist/**/*.spec.*",
		"!dist/**/*.test.*"
	],

Perhaps this is the way to ignore more files?

@lafkpages
Copy link

Perhaps this is the way to ignore more files?

Nope, that didn't work.

@dominikg
Copy link
Member

dominikg commented Jun 1, 2023

please reread the comments above regarding .npmignore and how it interacts with files.

for questions use https://svelte.dev/chat ( #svelte-and-kit ) or open a discussion on https://github.com/sveltejs/kit/discussions

If you have found a reproducible bug that is caused by svelte-package itself and not npm, please file that using the new issue workflow, including a link to a minimal reproduction.

the comment section of this PR is not the right place for this.

@oneezy
Copy link

oneezy commented Sep 16, 2023

@rob-balfre

Trying to upgrade svelte-select but when I run npx svelte-migrate package it errors...
Must of had an old migrate version somewhere... found a workaround:

$ npm i -g svelte-migrate 
$ npx svelte-migrate package

I believe you can just add @latest to fix:

npx svelte-migrate@latest 

Check this link for more details:
https://svelte.dev/docs/v4-migration-guide#:~:text=npx%20svelte%2Dmigrate%40latest%20svelte%2D4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change pkg:svelte-package Issues related to svelte-package
Projects
None yet