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

fix: make prepare and normalize have feature parity with legacy packages #36

Merged
merged 2 commits into from
Jun 6, 2023

Conversation

lukekarrys
Copy link
Contributor

@lukekarrys lukekarrys commented Jun 1, 2023

Due to npm/cli#6470, I added read-package-json and read-package-json-fast to this repo as dev deps to run the new tests from #31 against those packages. I found a few discrepancies which I fixed in this PR.

Full list of changes are as follows:

normalize / read-package-json-fast

  • convert bundleDependencies: false to []

prepare / read-package-json

  • ignore directories.bin if bin is present
  • walk up filesystem to look for git directory
  • log a warning for inaccessible bin entries but dont delete

These changes will be tracked in npm/statusboard#487 to see if they should be made as breaking changes with npm 10.

These legacy tests can be removed once we have decided to make these breaking changes.

Ref: npm/cli#6470
Fixes: #35

@lukekarrys lukekarrys requested a review from a team as a code owner June 1, 2023 17:06
@lukekarrys lukekarrys changed the title fix: ignore directories.bin if bin is present fix: make prepare have feature parity with legacy read-package-json Jun 1, 2023
@lukekarrys lukekarrys changed the title fix: make prepare have feature parity with legacy read-package-json fix: make prepare and normalize have feature parity with legacy packages Jun 1, 2023
…ackages

Due to npm/cli#6470, I added `read-package-json` and
`read-package-json-fast` to this repo as dev deps to run the new tests
from #31 against those packages. I found a few discrepancies which I
fixed in this PR.

Full list of changes are as follows:

**`normalize` / `read-package-json-fast`**
- convert `bundleDependencies: false` to `[]`

**`prepare` / `read-package-json`**
- ignore `directories.bin` if `bin` is present
- walk up filesystem to look for git directory
- log a warning for inaccessible bin entries but dont delete

These changes will be tracked in npm/statusboard#487 to see if they
should be made as breaking changes with npm 10.

These legacy tests can be removed once we have decided to make these
breaking changes.

Ref: npm/cli#6470
Fixes: #35
lib/normalize.js Outdated Show resolved Hide resolved
} catch {
delete data.bin[key]
log.warn('package-json', pkgId, `No bin file found at ${data.bin[key]}`)
// XXX: should a future breaking change delete bin entries that cannot be accessed?
Copy link
Member

@wraithgar wraithgar Jun 6, 2023

Choose a reason for hiding this comment

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

No, if anything we should throw and refuse to accept it.

@lukekarrys lukekarrys merged commit 7fcfa5a into main Jun 6, 2023
@lukekarrys lukekarrys deleted the lk/bins-and-bin branch June 6, 2023 17:27
@github-actions github-actions bot mentioned this pull request Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Release 3.1.0 does not correctly normalize package bins
2 participants