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

feat(pluginsdk-peerdeps): Create a separate package for managing peer dependencies for plugins #8637

Merged
merged 5 commits into from
Oct 12, 2020

Conversation

christopherthielen
Copy link
Contributor

@christopherthielen christopherthielen commented Oct 8, 2020

Today, plugins depend on @spinnaker/pluginsdk which:

  1. provides opinions about tooling
  2. provides the plugin with transitive dependencies via peerDependencies

This PR splits #2 into a separate npm package @spinnaker/pluginsdk-peerdeps.

Now, a plugin depends on both @spinnaker/pluginsdk and @spinnaker/pluginsdk-peerdeps. This is "enforced" using the yarn check-plugin tooling in pluginsdk.

The peerDependencies in@spinnaker/pluginsdk-peerdeps are managed using its package.json scripts.

  • First, a new plugin is scaffolded in a temp directory.
  • Then, the Deck core developer updates that scaffolded plugin's package versions.
  • Then, the versions are synchronized back to the @spinnaker/pluginsdk-peerdeps package.json.
  • Finally, the @spinnaker/pluginsdk-peerdeps package is published to NPM.

@@ -0,0 +1 @@
.scaffolddir
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The temp directory created for the scaffolded plugin is written into a file .scaffolddir

Comment on lines +6 to +11
"scripts": {
"usage": "cat USAGE.txt",
"stage": "npm run clean && ./sync-stage.sh",
"sync": "./sync-save.js",
"clean": "./clean.sh"
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These package scripts are written to help maintain this package.
They make it easier to update the peerDependencies

Comment on lines +13 to +25
"peerDependencies": {
"@spinnaker/core": "0.0.507",
"@spinnaker/pluginsdk": "0.0.21",
"@uirouter/core": "6.0.4",
"@uirouter/react": "1.0.2",
"lodash-es": "4.17.15",
"prop-types": "15.6.1",
"react": "16.8.6",
"react-dom": "16.8.6",
"rxjs": "5.4.2",
"@rollup/plugin-commonjs": "15.1.0",
"@rollup/plugin-node-resolve": "9.0.0",
"@rollup/plugin-typescript": "6.0.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are the packages+versions of both regular dependencies and devDependencies, all mixed together.

Comment on lines +47 to +55
"peerDevDependencies": [
"@rollup/plugin-commonjs",
"@rollup/plugin-node-resolve",
"@rollup/plugin-typescript",
"@spinnaker/eslint-plugin",
"@types/react",
"@typescript-eslint/eslint-plugin",
"@typescript-eslint/parser",
"bufferutil",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This list disambiguates the regular vs devDependencies in the list of peer dependencies above.

Comment on lines +1 to +4
#!/usr/bin/env node
/* @ts-check */
/* eslint-disable no-console */
/* Synchronizes versions of all (non-dev) dependencies from Deck's root package.json */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

After a plugin is scaffolded, this script copies the versions from deck's package.json, but only for regular dependencies. This is used to keep the version of react up to date, for example.

Comment on lines +24 to +35
const versionToUse = sourcesOfTruth
.map((path) => JSON.parse(fs.readFileSync(path).toString()))
.reduce((packages, pkgJson) => {
const { peerDependencies = {}, devDependencies = {}, dependencies = {} } = pkgJson;

return {
...packages,
...peerDependencies,
...devDependencies,
...dependencies,
};
}, {});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If a dependency is duplicated, this code prefers:

  • dependencies
  • devDependencies
  • peerDependencies

It also allows other sources of truth besides the main Deck package.json. In the future, dependencies will be moved from Deck into @spinnaker/core for example


fs.writeFileSync(packageJsonPath, JSON.stringify(packageJson, null, 2));

console.log(`Synchronized non-dev peer peerDependencies to ${packageJsonPath} from Deck`);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

todo: print the changes

Comment on lines +1 to +3
#!/usr/bin/env node
/* @ts-check */
/* eslint-disable no-console */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This script synchronizes the contents of the scaffolded plugin's package.json and re-writes the peerDependencies and devPeerDependencies of the @spinnaker/pluginsdk-peerdeps package.

Comment on lines +1 to +5
#!/usr/bin/env bash
set -e
[[ -e .scaffolddir ]] || {
echo Nothing to clean...
exit 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

clean up the temporary directory, because why not be nice?

Comment on lines -44 to -60
"peerDependencies": {
"@rollup/plugin-commonjs": "15.0.0",
"@rollup/plugin-node-resolve": "9.0.0",
"@rollup/plugin-typescript": "6.0.0",
"@spinnaker/core": "0.0.507",
"@spinnaker/eslint-plugin": "1.0.7",
"@types/react": "16.8.25",
"@typescript-eslint/eslint-plugin": "2.26.0",
"@typescript-eslint/parser": "2.26.0",
"@uirouter/core": "6.0.4",
"@uirouter/react": "1.0.2",
"eslint": "6.8.0",
"eslint-config-prettier": "6.10.1",
"eslint-plugin-react-hooks": "3.0.0",
"husky": "4.2.5",
"lodash-es": "4.17.15",
"npm-run-all": "4.1.5",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are the previous peerDependencies that got moved into pluginsdk-peerdeps

Comment on lines +38 to +51
// const latestPeerDepsVersion = getLatestPackageVersion(PEER_DEPS);
const latestPeerDepsVersion = getLatestPackageVersion(PLUGIN_SDK);
const installedPeerDepsVersion = getInstalledPackageVersion(pkgJson, PEER_DEPS);

report(
installedPeerDepsVersion
? `This plugin uses an out of date ${PEER_DEPS}@${installedPeerDepsVersion}`
: `This plugin does not have ${PEER_DEPS} installed`,
installedPeerDepsVersion === latestPeerDepsVersion,
{
description: `Install ${PEER_DEPS}@${latestPeerDepsVersion}`,
command: `yarn add ${PEER_DEPS}@${latestPeerDepsVersion}`,
},
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the check-plugin script to ensure the plugin has the most recent versions of both @spinnaker/pluginsdk AND @spinnaker/pluginsdk-peerdeps

@mergify mergify bot merged commit c0897f1 into spinnaker:master Oct 12, 2020
@christopherthielen christopherthielen deleted the pluginsdk-peerdeps branch October 12, 2020 20:32
christopherthielen added a commit to christopherthielen/deck that referenced this pull request Nov 10, 2020
feat(pluginsdk): Add 'yarn lint' script to package.json ([30fd60d](spinnaker@30fd60d))
feat(promiselike): Add then() and finally() methods to PromiseLike interface ([5886373](spinnaker@5886373))
feat(pluginsdk-peerdeps): Create a separate package for managing peer dependencies for plugins [spinnaker#8637](spinnaker#8637) ([c0897f1](spinnaker@c0897f1))
fix(pluginsdk): Remove package.json 'module' lint rule from check-plugin [spinnaker#8635](spinnaker#8635) ([98a16ea](spinnaker@98a16ea))
feat(pluginsdk): Add cli flags to scaffold.js to allow non-interactive scaffolding [spinnaker#8636](spinnaker#8636) ([5c84d68](spinnaker@5c84d68))
christopherthielen added a commit to christopherthielen/deck that referenced this pull request Nov 10, 2020
feat(pluginsdk): Add 'yarn lint' script to package.json ([30fd60d](spinnaker@30fd60d))
feat(promiselike): Add then() and finally() methods to PromiseLike interface ([5886373](spinnaker@5886373))
feat(pluginsdk-peerdeps): Create a separate package for managing peer dependencies for plugins [spinnaker#8637](spinnaker#8637) ([c0897f1](spinnaker@c0897f1))
fix(pluginsdk): Remove package.json 'module' lint rule from check-plugin [spinnaker#8635](spinnaker#8635) ([98a16ea](spinnaker@98a16ea))
feat(pluginsdk): Add cli flags to scaffold.js to allow non-interactive scaffolding [spinnaker#8636](spinnaker#8636) ([5c84d68](spinnaker@5c84d68))
christopherthielen added a commit to christopherthielen/deck that referenced this pull request Nov 10, 2020
feat(pluginsdk): Add 'yarn lint' script to package.json ([30fd60d](spinnaker@30fd60d))
feat(promiselike): Add then() and finally() methods to PromiseLike interface ([5886373](spinnaker@5886373))
feat(pluginsdk-peerdeps): Create a separate package for managing peer dependencies for plugins [spinnaker#8637](spinnaker#8637) ([c0897f1](spinnaker@c0897f1))
fix(pluginsdk): Remove package.json 'module' lint rule from check-plugin [spinnaker#8635](spinnaker#8635) ([98a16ea](spinnaker@98a16ea))
feat(pluginsdk): Add cli flags to scaffold.js to allow non-interactive scaffolding [spinnaker#8636](spinnaker#8636) ([5c84d68](spinnaker@5c84d68))
mergify bot pushed a commit that referenced this pull request Nov 10, 2020
* chore(pluginsdk): publish 0.0.22
feat(pluginsdk): Add 'yarn lint' script to package.json ([30fd60d](30fd60d))
feat(promiselike): Add then() and finally() methods to PromiseLike interface ([5886373](5886373))
feat(pluginsdk-peerdeps): Create a separate package for managing peer dependencies for plugins [#8637](#8637) ([c0897f1](c0897f1))
fix(pluginsdk): Remove package.json 'module' lint rule from check-plugin [#8635](#8635) ([98a16ea](98a16ea))
feat(pluginsdk): Add cli flags to scaffold.js to allow non-interactive scaffolding [#8636](#8636) ([5c84d68](5c84d68))

* fix(pluginsdk): Fix peer deps lint rule

* chore(pluginsdk): release 0.0.23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Reviewed and ready for merge target-release/1.23
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants