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

'lerna publish' doesn't respect publishConfig #404

Closed
5 tasks done
artechventure opened this issue Oct 31, 2022 · 6 comments · Fixed by #415
Closed
5 tasks done

'lerna publish' doesn't respect publishConfig #404

artechventure opened this issue Oct 31, 2022 · 6 comments · Fixed by #415
Labels
enhancement New feature or request feature has PR

Comments

@artechventure
Copy link
Contributor

Describe the bug

'lerna publish' doesn't respect publishConfig in package.json.

// package.json
{
  "main": "./src/index.ts",
  "publishConfig": {
    "main": "./dist/index.js"
  }
}

When published to npm via lerna publish,
"main" in package.json should be same with publishConfig.main
(yarn pack does do that, at least)
but it doesn't change when using lerna publish

Expectation

"main" in package.json should be same with publishConfig.main

{
  "main": "./dist/index.js"
}

Reproduction

  1. set "main" field in your package.json publishConfig.
  2. run lerna publish
  3. check the packed tgz and see if "main" in package.json changed

Lerna config and logs

lerna.json

<!-- Please paste your `lerna.json` here -->

lerna-debug.log

<!-- If you have a `lerna-debug.log` available, please paste it here -->
<!-- Otherwise, feel free to delete this <details> block -->

Environment Info

System:
    OS: macOS 12.1
    CPU: (10) arm64 Apple M1 Max
  Binaries:
    Node: 16.14.0 - /private/var/folders/zr/3tkznhqs6sn26wsynpczk2pxz0g_by/T/xfs-00da4926/node
    Yarn: 3.2.1 - /private/var/folders/zr/3tkznhqs6sn26wsynpczk2pxz0g_by/T/xfs-00da4926/yarn
  Utilities:
    Git: 2.32.1 - /usr/bin/git

Used Package Manager

yarn >= 2.x

Validations

@artechventure
Copy link
Contributor Author

https://github.com/ghiscoding/lerna-lite/blob/ec49df473ac38f6e069650acdd8edc0ce022ab21/packages/publish/src/lib/pack-directory.ts#L47

Yarn, pnpm has its own packing logic, (via yarn pack, pnpm pack).
And inside the pack command, there's a logic that overrides package.json's field by publishConfig.
So creating tar like this might not work as expected in yarn, pnpm.

@artechventure
Copy link
Contributor Author

lerna/lerna#3402

@ghiscoding
Copy link
Member

ghiscoding commented Oct 31, 2022

As you found out, the code is the same as Lerna, I'm not really sure what to do this with this. You seem to have more knowledge, can you create a PR that would address your problem? Hopefully not too much code specific to each package manager, the less specific code, the more generic code, the better.

@artechventure
Copy link
Contributor Author

artechventure commented Oct 31, 2022

I think best solution is getting tarballs by packageManager's own command.
If package manager's specific command is abstracted to this function, I think it should be not a problem.

import {tempfile} from '@lerna-lite/core';

async function createTarball(npmClient: 'pnpm' | 'yarn' | 'npm', pkg: Package) {
  const dir = tempfile();

  switch (npmClient) {
    case 'yarn':
      await execa('yarn', ['pack', '--out', `${dir}/%s-%v.tgz`]);
    default:
      await execa(npmClient, ['pack', '--pack-destination', dir]);
  }

  return `${dir}/${getTarballName(pkg)}`;
}

@ghiscoding
Copy link
Member

ghiscoding commented Oct 31, 2022

Lerna has always created all the package tarball by itself, they must have good reason to do it this way. They have pkg.contents defined in the tarball creation, it must be provided for good reason. I don't have full knowledge of why it was created this way and what it contains, so I'm not sure replacing the entire process is a good idea at this point. Perhaps doable under a new flag on the Lerna publish command.

https://github.com/ghiscoding/lerna-lite/blob/ec49df473ac38f6e069650acdd8edc0ce022ab21/packages/publish/src/lib/pack-directory.ts#L47-L63

However, I would rather try to avoid having package manager specific code, it's a lot less portable than generic code that works for all type of package managers. That's probably the reason they use the tar (aka node-tar) npm dependency. Also I wonder if going with package manager specific code would conflict with what Lerna really publishes, Lerna sometime modifies the package.json to be published and I wonder if we go with specific manager command would that drop the changes that Lerna applied in the publish command? For example when the user uses workspace: protocol then Lerna replaces these prefixes with the real pkg version, would that still work if we go with the code you suggest or will that be gone? Another good example of that is the new option --remove-package-fields (in PR #359) that I added in latest release to the lerna publish command. So anyway, I'm not saying it's not doable, but we have to be cautious on changing or adding new features like this.

https://github.com/ghiscoding/lerna-lite/blob/ec49df473ac38f6e069650acdd8edc0ce022ab21/packages/publish/src/publish-command.ts#L260-L298

Note

As a reminder, my main goal with this fork was to update all of Lerna's dependencies and fix all security related issues (I now use Renovate to update them once a week) and add some new features when possible. My main goal was achieved, however, I'm still learning the lib itself, there's still a lot of things that I don't quite understand and need help from the community (you for example) to understand how certain things work the way they do, so that we can improve when possible.

@artechventure
Copy link
Contributor Author

@ghiscoding I agree. I reported an issue in original leran repo, so it will be better to wait for their action(or not).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature has PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants