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

[package] remove tooling specific settings in publishConfig from generated package.json #5848

Merged
merged 6 commits into from Aug 15, 2022

Conversation

boyeborg
Copy link
Contributor

@boyeborg boyeborg commented Aug 8, 2022

I know you're currently doing a big rewrite. This is not an important fix, no rush in merging it.

Closes #5830

TL;DR: Removes publishConfig.directory from the generated package.json when using the package command. This setting is usually added to tell monorepo tools or publishing tools where you're package is located, i.e. the output directory of the package command. Copying that over to the generated package.json file does not make contextual sense, since the referenced directory don't exist, and breaks some monorepo tools.

I work in several monorepos where we have a wokspace based setup (usually using pnpm). In those monorepos we have several svelte packages created by the SvelteKit package command. Usually when you run this setup, you can use the workspace protocol to locally link one of those packages to an application living inside your monorepo (e.g. linking the charting package to the admin dashboard app). However, since SvelteKit's package command generates a new folder structure with its own package.json file, this does not work out of the box. PNPM has however recently added a feature (or fixed an old one, depending on how you look at it) that fixes this problem. It allows you to add a new location for your outputted package within the original package.json file using the publishConfig.directory key. By setting that to the package command's output directory, pnpm will link everything correctly. However, the package command copies the publishConfig.directory setting to the generated package.json file as well. This might cause some issues, since the referenced directory does not exist. It only exist in context of the original package.json. If you for example try to publish the package, you might get errors when the publishing tool does not find the referenced directory.

This PR tries to fix that potential issue by explicitly removing the publishConfig.directory key from the generated package.json file if it exists. However, this might not always yield the expected result. There might exist cases where you want to use that setting in the generated package that I haven't thought of.

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. All changesets should be patch until SvelteKit 1.0

@changeset-bot
Copy link

changeset-bot bot commented Aug 8, 2022

🦋 Changeset detected

Latest commit: e745544

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

@dummdidumm
Copy link
Member

Thank you! PR looks good, two things:

  • Is there anything else in publishConfig that could make problems, so should the whole object be removed?
  • We should mention that in the README that this field is removed so if someone needs this field they can at least know this from consulting the docs after being confused

@boyeborg
Copy link
Contributor Author

boyeborg commented Aug 9, 2022

@dummdidumm Thanks!
I don't think we should remove the whole object, since you can set things like custom npm registry and access there, that make sense to copy over to the generated package. The official docs from NPM only states that you can set whatever config options that are available for the npm publish command. So I guess that's mostly --registry, --access and --tag. All of which seems reasonable to keep.
The pnpm docs has a couple of extra uses for the publishConfig object (including directory). Most of them are fields you can set that pnpm uses to override the published package.json with when publishing. I guess that if you set any of those, and also use svelte-kit package to package the code you know what you're doing, and do it for a reason, so I don't think those should be removed.
But, since we're removing the publishConfig.directory, I think it also makes sense to remove the publishConfig.linkDirectory, since that's linked to the directory setting, and don't make sense without it. I'll add that to the list of keys. I'll also add the changes to the docs.

@benmccann benmccann added the pkg:svelte-package Issues related to svelte-package label Aug 9, 2022
@boyeborg boyeborg changed the title remove publishConfig.directory (if it exists) from generated package.json [package] remove tooling specific settings in publishConfig from generated package.json Aug 11, 2022
@Rich-Harris Rich-Harris merged commit b31759f into sveltejs:master Aug 15, 2022
@Rich-Harris
Copy link
Member

thanks!

@futantan
Copy link

futantan commented Aug 17, 2022

hi @boyeborg I noticed that you are using svelte-kit package in monorepo, I'd link to ask you for help: How can we publish it to the npm because svelte-kit package generate a new folder contains a package.json file

.
├── README.md
├── package <-- here is what we what to publish
├── src
├── package.json
├── svelte.config.js
└── tsconfig.json

We want to use this in monorepo, so in the top level package.json of this npm package, I added the following:

  "main": "package/index.js",
  "svelte": "package/index.js",

the question is how can I use changeset publish of the monorepo to publish the generated package folder?

@boyeborg
Copy link
Contributor Author

boyeborg commented Aug 17, 2022

@futantan I'm not too familiar with how changeset work, but the way we do it is to add this to the package.json of all packages we use svelte-kit package on:

"publishConfig": {
    "directory": "package",
    "linkDirectory": true
}

By having the publishConfig.directory set to point to the svelte-kit package output directory, all tooling we use understand that that's the home of the distributable package. If the changeset CLI doesn't use publishConfig.directory, I would encourage you to open an issue asking about it in the changeset repo.

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

Successfully merging this pull request may close these issues.

Package command copies publishConfig.directory into the generated package.json
5 participants