es-tooling / ecosystem-cleanup

A place to keep track of ongoing efforts to clean up the JS ecosystem
358 stars 2 forks source link

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

Open 43081j opened 7 months ago

43081j commented 7 months ago

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 commented 7 months ago

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

mehulkar commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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)

43081j commented 2 months ago

FYI there's also now fd-package-json: https://www.npmjs.com/package/fd-package-json

which may come in useful and is smaller than the now deprecated read-package-json-fast

AbhiPrasad commented 2 months ago

We identified git-log-parser as a big dependency to clean up in semantic-release as well - tracked with https://github.com/43081j/ecosystem-cleanup/issues/59

upstream issue: https://github.com/semantic-release/semantic-release/issues/3372

AbhiPrasad commented 2 months ago

newer upstream discussion: https://github.com/semantic-release/semantic-release/discussions/3376