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

migrate semantic-release to read-package-json-fast #20

Open
43081j opened this issue Jan 29, 2024 · 5 comments
Open

migrate semantic-release to read-package-json-fast #20

43081j opened this issue Jan 29, 2024 · 5 comments
Labels
help wanted Extra attention is needed

Comments

@43081j
Copy link
Owner

43081j commented Jan 29, 2024

semantic-release can be found here:
https://github.com/semantic-release/npm

it currently uses read-pkg here:
https://github.com/semantic-release/npm/blob/0154fb84631cd7debd4bcd0d78824386b76ae8c1/lib/get-pkg.js#L2

we can instead use read-package-json-fast:
https://github.com/npm/read-package-json-fast

its near enough 10x smaller, is maintained by the NPM team directly, and is more standard because of that.

@mehulkar
Copy link

PR is up semantic-release/npm#745! tests pass, but the library doesn't have too many recent commits, so hopefully we'll get a reply!

@mehulkar
Copy link

mehulkar commented Jan 31, 2024

Sadly read-pkg is still part of the module tree after this via (read-pkg -> read-pkg-up -> @semantic-release/release-notes-generator -> semantic-release)

➜  semantic-release-npm git:(mk/read-pkg) npm explain read-pkg                                 
read-pkg@9.0.1 dev
node_modules/read-pkg
  read-pkg@"^9.0.0" from @semantic-release/npm@11.0.2
  node_modules/@semantic-release/npm
    @semantic-release/npm@"^11.0.0" from semantic-release@23.0.0
    node_modules/semantic-release
      dev semantic-release@"23.0.0" from the root project
      peer semantic-release@">=20.1.0" from @semantic-release/commit-analyzer@11.1.0
      node_modules/@semantic-release/commit-analyzer
        @semantic-release/commit-analyzer@"^11.0.0" from semantic-release@23.0.0
      peer semantic-release@">=20.1.0" from @semantic-release/github@9.2.6
      node_modules/@semantic-release/github
        @semantic-release/github@"^9.0.0" from semantic-release@23.0.0
      peer semantic-release@">=20.1.0" from @semantic-release/npm@11.0.2
      peer semantic-release@">=20.1.0" from @semantic-release/release-notes-generator@12.1.0
      node_modules/@semantic-release/release-notes-generator
        @semantic-release/release-notes-generator@"^12.0.0" from semantic-release@23.0.0
  read-pkg@"^9.0.0" from read-pkg-up@11.0.0
  node_modules/read-pkg-up
    read-pkg-up@"^11.0.0" from @semantic-release/release-notes-generator@12.1.0
    node_modules/@semantic-release/release-notes-generator
      @semantic-release/release-notes-generator@"^12.0.0" from semantic-release@23.0.0
    read-pkg-up@"^11.0.0" from semantic-release@23.0.0
    node_modules/semantic-release
      dev semantic-release@"23.0.0" from the root project
      peer semantic-release@">=20.1.0" from @semantic-release/commit-analyzer@11.1.0
      node_modules/@semantic-release/commit-analyzer
        @semantic-release/commit-analyzer@"^11.0.0" from semantic-release@23.0.0
      peer semantic-release@">=20.1.0" from @semantic-release/github@9.2.6
      node_modules/@semantic-release/github
        @semantic-release/github@"^9.0.0" from semantic-release@23.0.0
      peer semantic-release@">=20.1.0" from @semantic-release/npm@11.0.2
      peer semantic-release@">=20.1.0" from @semantic-release/release-notes-generator@12.1.0
      node_modules/@semantic-release/release-notes-generator
        @semantic-release/release-notes-generator@"^12.0.0" from semantic-release@23.0.0

@43081j
Copy link
Owner Author

43081j commented Jan 31, 2024

ah because they also use read-pkg-up?

i had a look at how npm do that, they seem to just pair two packages: walk-up-path, and read-package-json-fast

i suspect you don't actually need a package for it anymore. for example:

const fileExists = (...p) => stat(resolve(...p))
  .then((st) => st.isFile())
  .catch(() => false);

for (const path of walkUp(cwd)) {
  const hasPackageJson = await fileExists(path, 'package.json');
  if (hasPackageJson) {
    return await readPackage(path);
  }
}

personally i'd take that over a deep chain of packages, but not all maintainers will want to maintain their own for loop for it.

@mehulkar
Copy link

ah because they also use read-pkg-up?

not in @semantic-release/npm. The dep tree looks like this:

@semantic-release/npm
|__semantic-release
    |__@semantic-release/release-notes-generator
        |__read-pkg-up
            |__read-pkg

@43081j
Copy link
Owner Author

43081j commented Jan 31, 2024

seems to come from here:
https://github.com/semantic-release/release-notes-generator/blob/29087a24af4d27783b387011b57c5f6d8e40d998/index.js#L75

it does seem a shame having to replace that with an equivalent "readPkgUp" function they would have to maintain (the for loop example in my last comment)

but looking through equivalent packages, there's not many that do this. npm themselves and most other reliable repos just walk up using a separate package (e.g. walk-up-path)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants