Skip to content

Conversation

@bajtos
Copy link
Member

@bajtos bajtos commented Nov 19, 2020

The first commit adds a new step to bin/check-package-metadata.js to ensure that all public packages list deps in the following order:

  1. peerDependencies
  2. dependencies
  3. devDependencies

The second commits adds a new step to bin/fix-monorepo.js to update all
public packages to list deps in the correct order.

The second commit fixes packages via bin/fix-monorepo.js to pass the new check enforced by bin/check-package-metadata.js.

This is a follow-up for the discussion in #6288.

To keep the changes small, focused and easy to review, I am intentionally NOT fixing dependencies vs. peerDependencies. I would like to open another PR to add an automated check + fix any violations discovered (e.g. in examples/greeter-extension).

Checklist

  • DCO (Developer Certificate of Origin) signed in all commits
  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

👉 Check out how to submit a PR 👈

@bajtos bajtos added the Internal Tooling Issues related to our tooling and monorepo infrastructore label Nov 19, 2020
@bajtos bajtos requested a review from raymondfeng November 19, 2020 09:54
@bajtos bajtos self-assigned this Nov 19, 2020
` Expected: ${expectedStr}`,
);
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Example output:

Error: Some of the packages are not following loopback-next guidelines:                                |Error: Some of the packages are not following loopback-next guidelines:
                                                                                                       |
- docs/package.json has incorrect order of keys.                                                       |- docs/package.json has incorrect order of keys.
  Actual:   devDependencies dependencies                                                               |  Actual:   devDependencies dependencies
  Expected: dependencies devDependencies                                                               |  Expected: dependencies devDependencies
- examples/greeter-extension/package.json has incorrect order of keys.                                 |- examples/greeter-extension/package.json has incorrect order of keys.
  Actual:   devDependencies dependencies                                                               |  Actual:   devDependencies dependencies
  Expected: dependencies devDependencies

@raymondfeng
Copy link
Contributor

I also would like to add a step in bin/fix-monorepo.js to fix the order of such properties in package.json.

@bajtos
Copy link
Member Author

bajtos commented Nov 20, 2020

I also would like to add a step in bin/fix-monorepo.js to fix the order of such properties in package.json.

That's a great idea, very helpful to resolve merge conflicts after some of the dependencies are updated by RenovateBot (as happened during my night) 👍

The implementation is a bit more tricky, because we need to decide where to put the block of dependency-related fields. I decided to start it after publishConfig, that option seems to be the most common case in other packages.

I reworked this PR as follows:

  1. The first commit modifying bin/check-package-metadata.js is unchanged.
  2. The second commit adds an auto-fix step to bin/fix-monorepo.js
  3. The third commit contains changes created by bin/fix-monorepo.js.

@raymondfeng LGTY now?

@bajtos bajtos requested a review from raymondfeng November 20, 2020 09:26
Add a new step to `bin/check-package-metadata.js` to ensure that all
public packages list deps in the following order:
1. `peerDependencies`
2. `dependencies
3. `devDependencies`

Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
Add a new step to `bin/fix-monorepo.js` to ensure that all
public packages list deps in the following order, starting
after `publishConfig` or `license` field:

    1. `peerDependencies`
    2. `dependencies
    3. `devDependencies`

Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
Ensure deps are specified in the following order, starting after
`publishConfig` or `license` field:

1. `peerDependencies`
2. `dependencies
3. `devDependencies`

Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
@bajtos bajtos merged commit ff5350c into master Nov 23, 2020
@bajtos bajtos deleted the fix/deps-order branch November 23, 2020 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Internal Tooling Issues related to our tooling and monorepo infrastructore

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants