kit icon indicating copy to clipboard operation
kit copied to clipboard

remove `publishConfig.directory` (if it exists) from generated package.json

Open boyeln opened this issue 2 years ago • 3 comments

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:

  • [x] 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
  • [x] This message body should clearly illustrate what problems it solves.
  • [ ] Ideally, include a test that fails without this PR but passes with it.

Tests

  • [x] Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • [x] 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

boyeln avatar Aug 08 '22 07:08 boyeln

🦋 Changeset detected

Latest commit: e745544a6384ce974db55959e641c652057d738f

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

changeset-bot[bot] avatar Aug 08 '22 07:08 changeset-bot[bot]

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

dummdidumm avatar Aug 08 '22 07:08 dummdidumm

@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.

boyeln avatar Aug 09 '22 06:08 boyeln

thanks!

Rich-Harris avatar Aug 15 '22 22:08 Rich-Harris

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?

futantan avatar Aug 17 '22 02:08 futantan

@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.

boyeln avatar Aug 17 '22 06:08 boyeln