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

Move into a single package.json #76412

Closed
8 tasks done
mistic opened this issue Sep 1, 2020 · 11 comments · Fixed by #80015
Closed
8 tasks done

Move into a single package.json #76412

mistic opened this issue Sep 1, 2020 · 11 comments · Fixed by #80015
Assignees
Labels
build chore Team:Operations Team label for Operations Team v7.11.0

Comments

@mistic
Copy link
Member

mistic commented Sep 1, 2020

The proposal is to use a single top level package.json (in the kibana folder) to manage all the dependencies we need in the workspace. Sub-directories package.json will still be found from now in order to keep defining and informing boundaries around plugins and packages from the core and also to allow to run specific npm scripts. In the future those sub directories package.json could maybe be removed and replaced with BUILD.bazel files which can both be used to inform boundaries and also to define nodejs scripts to be run with bazel instead of npm.

Benefits

  • Ability to easily and quickly have an overview about all the dependencies we are using across the project
  • Avoids installs of multiple versions of the same dependency
  • Helps avoiding install unwanted or non needed dependencies
  • Helps to keep track of extraneous and not declared dependencies (eslint was not correctly pick them before)
  • Makes security patches easier
  • Makes dependency upgrades easier
  • Possibly enforces a continuous work across teams to keep dependencies up to date and their code working with the latest versions
  • Removes the lock-in to a specifc package manager
  • Possibly makes node_modules installation faster as it simplifies the flattening algorithm
  • Improves the ability to cache node_modules on the CI independently of the package manager being used

Cons

  • Bigger root package.json
  • Makes it difficult to test a new package version in isolation
  • Could potentially introduce extra work on solution teams to work with the supported versions of a given package across the workspace

Steps

  • Condense every workspace dependency and devDependency under the root package.json
  • Remove dependencies and devDependencies declarations from sub directories package.json
  • Remove yarn workspaces support from @kbn/pm
  • Delete .yarnrc
  • Remove workspaces key from package.json
  • Local packages should be declared with link: instead of version in the root package.json
  • Rework link_project_executables.ts on @kbn/pm to link the root ./node_modules/.bin for every @kbn/pm project so we can run npm scripts on them
  • Implement build step to understand what are the oss dependencies so we can delete them from the OSS distributable build
@mistic mistic added chore Team:Operations Team label for Operations Team build v7.11.0 labels Sep 1, 2020
@mistic mistic self-assigned this Sep 1, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@mistic mistic mentioned this issue Sep 1, 2020
23 tasks
@kobelb
Copy link
Contributor

kobelb commented Sep 21, 2020

Would there be a single package.json, even considering @kbn packages within the packages/ folder?

/cc @peterschretlen @stacey-gammon

@tylersmalley
Copy link
Contributor

@kobelb, that is correct.

@peterschretlen
Copy link
Contributor

@tylersmalley @mistic just looking to clarify the motivation behind a single package.json - is it because Bazel requires it, or for the benefits listed in the description?

@mistic
Copy link
Member Author

mistic commented Sep 22, 2020

@peterschretlen I've talked with @tylersmalley about this offline. We believe both are present on that equation - we have started by looking at it because it was a blocker for our bazel exploration and then we discovered it might actually be a good idea anyway

@tylersmalley
Copy link
Contributor

@peterschretlen it's actually both. Bazel currently does not support Yarn workspaces, however, it is something they are working to support but there is no timeline for that. While investigating this, we found that there are numerous benefits to migrating away from Yarn Workspaces as listed in the description. In addition, we have experienced constant issues with Yarn which we believe is only relevant to Workspaces. An example of this is some developers encounter an unexpected issue which is only resolved by clearing the node_modules in the project, cleaning the Yarn cache, and re-running yarn install. We believe this is due to the wrong package being resolved.

I am working on generating a report to understand what dependencies have differing versions to understand the scope and what is needed.

@tylersmalley
Copy link
Contributor

tylersmalley commented Sep 22, 2020

Here is a report of our dependencies with different versions across packages. For the most part, seems like we have a lot of opportunities to reduce our overhead.

@elastic/elasticsearch (7.9.0-rc.2,7.9.0-rc.1)
  - package.json@7.9.0-rc.2
  - packages/kbn-es/package.json@7.9.0-rc.1
  - x-pack/plugins/apm/scripts/package.json@7.9.0-rc.1
@elastic/eui (29.0.0,0.0.55)
  - package.json@29.0.0
  - x-pack/package.json@29.0.0
  - packages/kbn-ui-framework/package.json@0.0.55
  - packages/kbn-ui-shared-deps/package.json@29.0.0
  - test/interpreter_functional/plugins/kbn_tp_run_pipeline/package.json@29.0.0
  - test/plugin_functional/plugins/kbn_sample_panel_action/package.json@29.0.0
  - test/plugin_functional/plugins/kbn_tp_custom_visualizations/package.json@29.0.0
angular-sanitize (^1.8.0,1.8.0)
  - package.json@^1.8.0
  - x-pack/package.json@1.8.0
chalk (^2.4.2,^4.1.0)
  - package.json@^2.4.2
  - x-pack/package.json@^4.1.0
  - packages/kbn-dev-utils/package.json@^4.1.0
  - packages/kbn-es/package.json@^4.1.0
  - packages/kbn-pm/package.json@^4.1.0
  - packages/kbn-test/package.json@^4.1.0
  - packages/kbn-ui-framework/package.json@^4.1.0
commander (3.0.2,^3.0.0)
  - package.json@3.0.2
  - x-pack/package.json@3.0.2
  - packages/kbn-spec-to-console/package.json@^3.0.0
glob-all (^3.2.1,^3.1.0)
  - package.json@^3.2.1
  - packages/kbn-eslint-import-resolver-kibana/package.json@^3.1.0
lodash (^4.17.20,^4.17.15)
  - package.json@^4.17.20
  - x-pack/package.json@^4.17.20
  - packages/kbn-config/package.json@^4.17.20
  - packages/kbn-interpreter/package.json@^4.17.15
  - packages/kbn-pm/package.json@^4.17.15
  - packages/kbn-std/package.json@^4.17.15
  - packages/kbn-telemetry-tools/package.json@^4.17.20
  - packages/kbn-test/package.json@^4.17.15
  - packages/kbn-ui-framework/package.json@^4.17.15
lru-cache (4.1.5,^4.1.5)
  - package.json@4.1.5
  - packages/kbn-eslint-import-resolver-kibana/package.json@^4.1.5
mustache (2.3.2,^2.3.0)
  - package.json@2.3.2
  - x-pack/package.json@^2.3.0
node-fetch (2.6.1,^2.6.1)
  - package.json@2.6.1
  - x-pack/package.json@^2.6.1
  - packages/kbn-es/package.json@^2.6.1
react-router (^5.2.0,^3.2.0)
  - package.json@^5.2.0
  - x-pack/package.json@^5.2.0
  - packages/kbn-ui-framework/package.json@^3.2.0
  - packages/kbn-ui-shared-deps/package.json@^5.2.0
redux-thunk (^2.3.0,2.2.0)
  - package.json@^2.3.0
  - x-pack/package.json@^2.3.0
  - packages/kbn-ui-framework/package.json@2.2.0
rison-node (1.0.2,0.3.1)
  - package.json@1.0.2
  - x-pack/package.json@0.3.1
rxjs (^6.5.5,6.5.5)
  - package.json@^6.5.5
  - x-pack/package.json@^6.5.5
  - packages/kbn-config/package.json@^6.5.5
  - packages/kbn-dev-utils/package.json@^6.5.5
  - packages/kbn-optimizer/package.json@^6.5.5
  - packages/kbn-pm/package.json@^6.5.5
  - packages/kbn-storybook/package.json@6.5.5
  - packages/kbn-test/package.json@^6.5.5
  - packages/kbn-ui-shared-deps/package.json@^6.5.5
semver (^5.5.0,5.7.0)
  - package.json@^5.5.0
  - x-pack/package.json@5.7.0
yauzl (2.10.0,^2.10.0)
  - package.json@2.10.0
  - packages/kbn-es/package.json@^2.10.0
@types/glob (^7.1.1,^5.0.35)
  - package.json@^7.1.1
  - x-pack/package.json@^7.1.1
  - packages/kbn-pm/package.json@^5.0.35
@types/globby (^8.0.0,^6.1.0)
  - package.json@^8.0.0
  - packages/kbn-pm/package.json@^6.1.0
@types/node (>=10.17.17 <10.20.0,^14.0.14)
  - package.json@>=10.17.17 <10.20.0
  - x-pack/package.json@>=10.17.17 <10.20.0
  - packages/kbn-pm/package.json@>=10.17.17 <10.20.0
  - x-pack/plugins/apm/e2e/package.json@^14.0.14
@types/strip-ansi (^3.0.0,^5.2.1)
  - package.json@^3.0.0
  - packages/kbn-pm/package.json@^3.0.0
  - packages/kbn-test/package.json@^5.2.1
archiver (^3.1.1,3.1.1)
  - package.json@^3.1.1
  - x-pack/package.json@3.1.1
getopts (^2.2.4,^2.2.5)
  - package.json@^2.2.4
  - packages/kbn-dev-utils/package.json@^2.2.5
  - packages/kbn-es/package.json@^2.2.4
  - packages/kbn-i18n/package.json@^2.2.4
  - packages/kbn-interpreter/package.json@^2.2.4
  - packages/kbn-pm/package.json@^2.2.4
  - packages/kbn-test/package.json@^2.2.4
history (^4.9.0,4.9.0)
  - package.json@^4.9.0
  - x-pack/package.json@4.9.0
ngreact (0.5.1,^0.5.1)
  - package.json@0.5.1
  - x-pack/package.json@^0.5.1
normalize-path (^3.0.0,3.0.0)
  - package.json@^3.0.0
  - packages/kbn-dev-utils/package.json@^3.0.0
  - packages/kbn-optimizer/package.json@^3.0.0
  - packages/kbn-plugin-generator/package.json@^3.0.0
  - packages/kbn-storybook/package.json@3.0.0
  - packages/kbn-telemetry-tools/package.json@^3.0.0
pngjs (^3.4.0,3.4.0)
  - package.json@^3.4.0
  - x-pack/package.json@3.4.0
react-redux (^7.2.0,^5.1.2)
  - package.json@^7.2.0
  - x-pack/package.json@^7.2.0
  - packages/kbn-ui-framework/package.json@^5.1.2
redux (^4.0.5,3.7.2)
  - package.json@^4.0.5
  - x-pack/package.json@^4.0.5
  - packages/kbn-ui-framework/package.json@3.7.2
simple-git (1.116.0,^1.91.0)
  - package.json@1.116.0
  - x-pack/package.json@1.116.0
  - packages/kbn-es/package.json@^1.91.0
strip-ansi (^3.0.1,^6.0.0,^4.0.0,^5.2.0)
  - package.json@^3.0.1
  - packages/kbn-dev-utils/package.json@^6.0.0
  - packages/kbn-pm/package.json@^4.0.0
  - packages/kbn-test/package.json@^5.2.0
tape (^4.13.0,^5.0.1)
  - package.json@^4.13.0
  - packages/elastic-safer-lodash-set/package.json@^5.0.1
axios (^0.19.0,^0.19.2)
  - x-pack/package.json@^0.19.0
  - x-pack/package.json@^0.19.0
  - packages/kbn-dev-utils/package.json@^0.19.0
  - packages/kbn-release-notes/package.json@^0.19.2
  - x-pack/plugins/apm/e2e/package.json@^0.19.2
extract-zip (^1.7.0,^2.0.1)
  - x-pack/package.json@^1.7.0
  - packages/kbn-plugin-helpers/package.json@^2.0.1
graphql (^0.13.2,^14.0.0)
  - x-pack/package.json@^0.13.2
  - packages/kbn-release-notes/package.json@^14.0.0
graphql-tag (^2.9.2,^2.10.3)
  - x-pack/package.json@^2.9.2
  - packages/kbn-release-notes/package.json@^2.10.3
raw-loader (3.1.0,^3.1.0)
  - x-pack/package.json@3.1.0
  - packages/kbn-monaco/package.json@3.1.0
  - packages/kbn-optimizer/package.json@^3.1.0
  - packages/kbn-ui-framework/package.json@^3.1.0
stats-lite (^2.2.0,2.2.0)
  - x-pack/package.json@^2.2.0
  - x-pack/test/plugin_api_perf/plugins/task_manager_performance/package.json@2.2.0
webpack (^4.41.5,^4.43.0)
  - x-pack/package.json@^4.41.5
  - packages/kbn-eslint-import-resolver-kibana/package.json@^4.41.5
  - packages/kbn-interpreter/package.json@^4.41.5
  - packages/kbn-monaco/package.json@^4.41.5
  - packages/kbn-optimizer/package.json@^4.41.5
  - packages/kbn-pm/package.json@^4.41.5
  - packages/kbn-storybook/package.json@^4.41.5
  - packages/kbn-ui-framework/package.json@^4.41.5
  - packages/kbn-ui-shared-deps/package.json@^4.41.5
  - x-pack/plugins/apm/e2e/package.json@^4.43.0
@cypress/webpack-preprocessor (^4.1.0,^5.4.1)
  - x-pack/package.json@^4.1.0
  - x-pack/plugins/apm/e2e/package.json@^5.4.1
cypress (5.0.0,^4.9.0)
  - x-pack/package.json@5.0.0
  - x-pack/plugins/apm/e2e/package.json@^4.9.0
mini-css-extract-plugin (0.8.0,0.7.0)
  - x-pack/package.json@0.8.0
  - packages/kbn-storybook/package.json@0.7.0
  - packages/kbn-ui-shared-deps/package.json@0.8.0
react-docgen-typescript-loader (^3.1.1,3.1.0)
  - x-pack/package.json@^3.1.1
  - packages/kbn-storybook/package.json@3.1.0
tmp (0.1.0,^0.1.0)
  - x-pack/package.json@0.1.0
  - packages/kbn-test/package.json@^0.1.0
ts-loader (^6.0.4,^7.0.5)
  - x-pack/package.json@^6.0.4
  - x-pack/plugins/apm/e2e/package.json@^7.0.5
yargs (^15.4.1,^15.4.0)
  - x-pack/package.json@^15.4.1
  - x-pack/plugins/apm/e2e/package.json@^15.4.0
tsd (^0.13.1,^0.7.4)
  - packages/elastic-safer-lodash-set/package.json@^0.13.1
  - packages/kbn-config/package.json@^0.7.4
  - packages/kbn-config-schema/package.json@^0.13.1
  - packages/kbn-std/package.json@^0.13.1
  - packages/kbn-utility-types/package.json@^0.13.1
abort-controller (^2.0.3,^3.0.0)
  - packages/kbn-es/package.json@^2.0.3
  - x-pack/plugins/ingest_manager/package.json@^3.0.0
url-loader (2.2.0,^2.2.0)
  - packages/kbn-interpreter/package.json@2.2.0
  - packages/kbn-optimizer/package.json@^2.2.0
node-sass (^4.13.0,^4.13.1)
  - packages/kbn-optimizer/package.json@^4.13.0
  - packages/kbn-ui-framework/package.json@^4.13.1
inquirer (^7.3.3,^1.2.2)
  - packages/kbn-plugin-generator/package.json@^7.3.3
  - packages/kbn-plugin-helpers/package.json@^1.2.2
@types/inquirer (^7.3.1,^6.5.0)
  - packages/kbn-plugin-generator/package.json@^7.3.1
  - packages/kbn-plugin-helpers/package.json@^6.5.0
ora (^1.4.0,^4.0.4)
  - packages/kbn-pm/package.json@^1.4.0
  - x-pack/plugins/apm/e2e/package.json@^4.0.4
The script for my future reference

const { readFileSync } = require('fs');
const globby = require('globby');

const packagePaths = globby.sync(['**/package.json'], {
  gitignore: true,
});

const dependencies = {};

function addPackage(name, version, packagePath) {
  if (version.match(/link:/)) return;

  if (!dependencies[name]) {
    dependencies[name] = [];
  }

  dependencies[name].push({
    path: packagePath,
    version,
  });
}

function checkPackages(packages, packagePath) {
  if (!packages) return;

  for (const [name, version] of Object.entries(packages)) {
    addPackage(name, version, packagePath);
  }
}

packagePaths.forEach((packagePath) => {
  try {
    const pkg = JSON.parse(readFileSync(packagePath));

    checkPackages(pkg.dependencies, packagePath);
    checkPackages(pkg.devDependencies, packagePath);
  } catch (e) {
    console.log(`failed to parse ${packagePath}`, e.message);
  }
});

Object.keys(dependencies).forEach((name) => {
  const dependency = dependencies[name];
  const unique = [...new Set(dependency.map((d) => d.version))];

  if (unique.length > 1) {
    console.log(`${name} (${unique.join(',')})`);
    dependency.forEach((d) => console.log(`  - ${d.path}@${d.version}`));
  }
});

@joshdover
Copy link
Contributor

joshdover commented Sep 23, 2020

To get a picture of how this would impact solution teams, we can break this out by plugins that are actually using workspaces. These are the only plugins using custom package versions:

  • x-pack/plugins/apm/scripts/package.json
    • @elastic/elasticsearch@7.9.0-rc.1
      • vs 7.9.0-rc.2 in the root package.json
  • x-pack/plugins/apm/e2e/package.json
    • @types/node@^14.0.14
      • vs >=10.17.17 <10.20.0 in the root package.json ⚠️
    • axios@^0.19.2
      • vs ^0.19.0 in the root package.json
    • webpack@^4.43.0
      • vs ^4.41.5 in x-pack/package.json
    • @cypress/webpack-preprocessor@^5.4.1
      • vs ^4.1.0 in x-pack/package.json ⚠️
    • cypress@^4.9.0
      • vs 5.0.0 in x-pack/package.json ⚠️

So today, none of our plugins are using custom versions of package in their actual product. We only have APM using a few custom versions in their custom tooling for cypress. Only 3 of these seem potentially problematic (indicated by the ⚠️).

However, I don't suspect these will be too bad to fix. The @types/node version is wrong in apm/e2e (we use Node v10, not v14). The cypress discrepancies will need to be resolved, but luckily only one other team is using cypress right now (Security Solution) so this not will have a large impact on many teams.

We used to have more teams using custom versions of packages than we do now. I think this is a good sign that this workspaces feature is no longer needed frequently and we can probably live without it for a while.

@mistic
Copy link
Member Author

mistic commented Oct 2, 2020

@joshdover @tylersmalley just to let you know that we merged #78825 so at the moment we no longer have multiple dependencies declared for the same dependency across the repository

@jfsiii
Copy link
Contributor

jfsiii commented Oct 14, 2020

Avoids installs of multiple versions of the same dependency

Will devs be able to use aliases to add, say, "pkg3": "pkg@3.x" while "pkg": "pkg@2.x" is used elsewhere?

I understand not wanting N versions with minimal differences but there have been a few occasions where this has been a critical feature.

@mistic
Copy link
Member Author

mistic commented Oct 14, 2020

@jfsiii I would say using alias would be available to be used for such occasions 😃 I felt how useful it can be in a couple of situations for example during the work to migrate lodash 3 to lodash 4 across the entire repo, so we plan to keep that ability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build chore Team:Operations Team label for Operations Team v7.11.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants