feat: sort dependencies according to detected package manager, support `devEngines` field by Tobbe · Pull Request #382 · keithamus/sort-package-json

Tobbe · 2025-12-03T11:00:53Z

Different package managers sort dependencies using different sorting orders. This PR tries to detect the package manager that's used by the project, and chooses the sorting order based on the detected package manager

Fixes #363

keithamus

I'd like @fisker to review this though, and perhaps @pfe-nazaries who also commented in #363

fisker · 2025-12-07T09:50:13Z

@Tobbe I pushed a refactor to simplify logic and reduce diff, can you check if there are adjustments (I didn't check comments) needed?

fisker · 2025-12-08T12:07:59Z

+const hasYarnOrPnpmLock = (packageJson) => {
+  if (!cache.has(packageJson)) {
+    cache.set(
+      packageJson,


@keithamus Do you think we should cache this based on process.cwd()?

Maybe also detect .yarn dir, .yarnrc.yml, pnpm-workspace.yaml

Sounds reasonable yes.

@Tobbe

The one thing that's left to resolve is the caching. There's a comment regarding using cwd that I don't fully understand. Was the idea to use only process.cwd(), or both process.cwd() and packageJson together?

You can memoize this function by the process.cwd(), as it should invariably return the same answer for a single execution. The packageJson is irrelevant as it's not an input to the question this function answers, only the directory is. So I would remove the packageJson arg and use a dir arg (and change the calls to check the dir + filename).

I had to add cache.clear() to the start of sortPackageJson() for tests to pass. Since they're all using the same dir they'll reuse the same cache entry

I could mock process.cwd() in the tests instead if you prefer

Tobbe · 2025-12-13T16:01:19Z

I've merged in the latest code from main and fixed merge conflicts.
I've also addressed the remaining review comments, except one.

The one thing that's left to resolve is the caching. There's a comment regarding using cwd that I don't fully understand. Was the idea to use only process.cwd(), or both process.cwd() and packageJson together?

fisker · 2025-12-13T20:57:14Z

I made a minor refactor 854af79 (#382)

fisker · 2025-12-13T21:03:58Z

 }

 function sortPackageJson(jsonIsh, options = {}) {
+  cache.clear()


If we are doing this, maybe just use a variable instead? (sorry, didn't notice this change)

But this makes the cache meaningless. Let's remove it, mock process.cwd() instead in test.

I don't think clearing the cache once at the start makes the cache meaningless. sortDependencies is called multiple times during one execution of sortPackageJson(), each of those sortDependencies invocations could potentially try to read the filesystem, which the cache still prevents even when clearing it once at the start

Tobbe · 2025-12-13T21:22:48Z

I made a minor refactor 854af79 (#382)

I did fs.existsSync(path.join(dir, 'yarn.lock')) etc because @keithamus wanted me to include both the dir and the filename, as you can see in his comment here: #382 (comment)

He also wanted me to pass the dir to the function (same comment, #382 (comment))

@fisker you have now removed both those changes. So I'll let you two decide what solution you want here 🙂

fisker · 2025-12-13T21:48:13Z

He also wanted me to pass the dir to the function

There is no difference between pass or call it in hasYarnOrPnpmFiles, but move it to hasYarnOrPnpmFiles we can omit path.join call.

fisker · 2025-12-13T21:54:35Z

+    packageJson.pnpm ||
+    (typeof packageManager === 'string' &&
+      (packageManager.startsWith('yarn@') ||
+        packageManager.startsWith('pnpm@')))


Maybe corepack also support {packageManager: 'npm@1.0.0'}?

Yeah, turns out packageManager can also be an object. bb0c487 (#382)

And there are devEngines.packageManager ...

9fab533 (#382)

Maybe corepack also support {packageManager: 'npm@1.0.0'}?

No, npm is not supported as a value for packageManager: nodejs/node#51888

…e: '...'}}`

fisker · 2025-12-13T23:04:57Z

      ? { ...object, [property]: over(object[property], ...args) }
-      : object
+      : object,
+  )


@keithamus This should be a bug, we didn't check the object is not null or undefined before, so it can crash. Some other changes removing onObject in this PR also related to this. Found this adding support for devEngines, do you want me to extract this part to separated PRs?

Nice find! We can keep it here.

Tobbe · 2025-12-13T23:14:04Z

+
+  // packageManager
+  t.deepEqual(
+    getDependencyOrders({ packageManager: 'npm@1.0.0' }, () => {


npm is not supported as a value for packageManager nodejs/node#51888

Should we block on this?

Block this PR you mean? No, I'd rather get it merged

Great! Merged then 😉

keithamus · 2025-12-14T08:41:24Z

      ? { ...object, [property]: over(object[property], ...args) }
-      : object
+      : object,
+  )


Nice find! We can keep it here.

github-actions · 2025-12-14T09:38:04Z

🎉 This PR is included in version 3.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

keithamus · 2025-12-14T09:38:10Z

Thanks for your work on this @Tobbe!

Tobbe · 2025-12-14T09:42:23Z

@keithamus and @fisker Thanks for guiding me through this 🙏

feat: sort dependencies according to project's package manager

1b43cdc

Tobbe mentioned this pull request Dec 3, 2025

chore(deps): update dependency prettier-plugin-packagejson to v2.5.20 cedarjs/cedar#604

Closed

1 task

keithamus previously approved these changes Dec 5, 2025

View reviewed changes

Comment thread index.js Outdated

sortObjectAlphabeticallyDetectingPackageManager

8ec225c

Tobbe dismissed keithamus’s stale review via 8ec225c December 7, 2025 07:30

shorter sort fn name

6a6c741

fisker reviewed Dec 7, 2025

View reviewed changes

Comment thread index.js Outdated

fisker reviewed Dec 7, 2025

View reviewed changes

Comment thread index.js Outdated

Tobbe and others added 4 commits December 7, 2025 10:37

rename sorting function

89b0657

Cache detected package manager

4249549

rename cache variable

170aa35

refactor: simplify

c689e21

fisker reviewed Dec 7, 2025

View reviewed changes

Comment thread index.js Outdated

fix: avoid fs access

e46502d

keithamus reviewed Dec 7, 2025

View reviewed changes

Comment thread index.js Outdated

Comment thread index.js Outdated

fisker reviewed Dec 8, 2025

View reviewed changes

Tobbe added 2 commits December 13, 2025 16:45

Merge branch 'main' into tobbe-feat-package-manager-specific-sort

08a6b10

Extend list of files/folders checked for detecting yarn and pnpm

921a274

Tobbe and others added 3 commits December 13, 2025 20:11

Memoize on dir

efdd62e

Sort catalog based on package manager

f010dc7

refactor: minor tweak

854af79

fisker previously approved these changes Dec 13, 2025

View reviewed changes

fisker reviewed Dec 13, 2025

View reviewed changes

fisker added 2 commits December 14, 2025 05:41

test: rewrite tests

7b39bf5

refactor: inline sortDependenciesLikeNpm

c9962ca

fisker dismissed their stale review via 7b39bf5 December 13, 2025 21:45

chore: don't return result from t.fail()

129a63a

fisker previously approved these changes Dec 13, 2025

View reviewed changes

fisker reviewed Dec 13, 2025

View reviewed changes

feat: support {packageManager: 'npm@...'} and `{packageManager:{nam…

bb0c487

…e: '...'}}`

fisker dismissed their stale review via bb0c487 December 13, 2025 22:00

fisker previously approved these changes Dec 13, 2025

View reviewed changes

fix: support devEngines.packageManager

9fab533

fisker dismissed their stale review via 9fab533 December 13, 2025 22:05

test: test devEngines

fd38d79

Tobbe commented Dec 13, 2025

View reviewed changes

Comment thread index.js Outdated

feat: support sort packageManager and devEngines

c909ddb

Tobbe commented Dec 13, 2025

View reviewed changes

Comment thread tests/dependency-sorting.js Outdated

Tobbe commented Dec 13, 2025

View reviewed changes

Comment thread tests/dependency-sorting.js Outdated

fisker added 2 commits December 14, 2025 06:45

fix: top packageManager can only be string

f4db109

docs: add devEngines to readme

e883f72

fisker changed the title ~~feat: sort dependencies according to detected package manager~~ feat: sort dependencies according to detected package manager, support devEngines field Dec 13, 2025

fisker previously approved these changes Dec 13, 2025

View reviewed changes

chore: remove useless onObject

3fb37b8

fisker dismissed their stale review via 3fb37b8 December 13, 2025 23:00

fisker approved these changes Dec 13, 2025

View reviewed changes

fisker reviewed Dec 13, 2025

View reviewed changes

Tobbe commented Dec 13, 2025

View reviewed changes

keithamus approved these changes Dec 14, 2025

View reviewed changes

keithamus merged commit e3f2370 into keithamus:main Dec 14, 2025
3 checks passed

github-actions Bot added the released label Dec 14, 2025

Tobbe mentioned this pull request Dec 14, 2025

chore(deps): Update prettier-plugin-packagejson and sort-package-json cedarjs/cedar#699

Merged

Conversation

Tobbe commented Dec 3, 2025

Uh oh!

keithamus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fisker commented Dec 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tobbe commented Dec 13, 2025

Uh oh!

fisker commented Dec 13, 2025

Uh oh!

fisker Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tobbe commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fisker commented Dec 13, 2025

Uh oh!

fisker Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fisker Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Dec 14, 2025

Uh oh!

keithamus commented Dec 14, 2025

Uh oh!

Tobbe commented Dec 14, 2025

fisker Dec 13, 2025 •

edited

Loading

Tobbe commented Dec 13, 2025 •

edited

Loading

fisker Dec 13, 2025 •

edited

Loading

fisker Dec 13, 2025 •

edited

Loading