diff --git a/.github/workflows/frontend.yml b/.github/workflows/frontend.yml index d916141588..55a594d55e 100644 --- a/.github/workflows/frontend.yml +++ b/.github/workflows/frontend.yml @@ -71,6 +71,9 @@ jobs: if: ${{ steps.yarn-lock.outcome == 'failure' }} run: yarn lerna -- run build + - name: verify type dependencies + run: yarn lint:type-deps + - name: test changed packages if: ${{ steps.yarn-lock.outcome == 'success' }} run: yarn lerna -- run test --since origin/master -- --coverage diff --git a/.github/workflows/master.yml b/.github/workflows/master.yml index a41f25c262..14619305cd 100644 --- a/.github/workflows/master.yml +++ b/.github/workflows/master.yml @@ -60,6 +60,9 @@ jobs: - name: build run: yarn build + - name: verify type dependencies + run: yarn lint:type-deps + - name: test run: yarn lerna -- run test -- --coverage diff --git a/docs/getting-started/development-environment.md b/docs/getting-started/development-environment.md index 4dd2f7c9c7..489116fcc8 100644 --- a/docs/getting-started/development-environment.md +++ b/docs/getting-started/development-environment.md @@ -44,6 +44,7 @@ yarn build # Build published versions of packages, depends on tsc yarn lint # lint packages that have changed since later commit on origin/master yarn lint:all # lint all packages +yarn lint:type-deps # verify that @types/* dependencies are placed correctly in packages yarn test # test packages that have changed since later commit on origin/master yarn test:all # test all packages diff --git a/package.json b/package.json index de924894c7..437ce36c4b 100644 --- a/package.json +++ b/package.json @@ -15,6 +15,7 @@ "test:all": "lerna run test -- --coverage", "lint": "lerna run lint --since origin/master --", "lint:all": "lerna run lint --", + "lint:type-deps": "node scripts/check-type-dependencies.js", "docker-build": "yarn bundle && docker build . -t spotify/backstage", "create-plugin": "backstage-cli create-plugin", "remove-plugin": "backstage-cli remove-plugin", diff --git a/packages/core-api/package.json b/packages/core-api/package.json index 73e7a18d33..a8c2b939ac 100644 --- a/packages/core-api/package.json +++ b/packages/core-api/package.json @@ -31,9 +31,7 @@ "@backstage/theme": "^0.1.1-alpha.6", "@material-ui/core": "^4.9.1", "@material-ui/icons": "^4.9.1", - "@types/jest": "^25.2.2", - "@types/node": "^12.0.0", - "@types/zen-observable": "^0.8.0", + "@types/react": "^16.9", "prop-types": "^15.7.2", "react": "^16.12.0", "react-router-dom": "^5.2.0", @@ -46,6 +44,9 @@ "@testing-library/jest-dom": "^5.7.0", "@testing-library/react": "^9.3.2", "@testing-library/user-event": "^10.2.4", + "@types/jest": "^25.2.2", + "@types/node": "^12.0.0", + "@types/zen-observable": "^0.8.0", "jest-fetch-mock": "^3.0.3" }, "files": [ diff --git a/packages/core-api/src/routing/index.ts b/packages/core-api/src/routing/index.ts index 98e4f46d98..67d4c82167 100644 --- a/packages/core-api/src/routing/index.ts +++ b/packages/core-api/src/routing/index.ts @@ -16,3 +16,4 @@ export * from './types'; export { createRouteRef } from './RouteRef'; +export type { MutableRouteRef } from './RouteRef'; diff --git a/packages/core/package.json b/packages/core/package.json index a89ffc0b92..452633a4e5 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -33,13 +33,8 @@ "@material-ui/core": "^4.9.1", "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.45", - "@types/classnames": "^2.2.9", - "@types/google-protobuf": "^3.7.2", - "@types/jest": "^25.2.2", - "@types/node": "^12.0.0", - "@types/react-helmet": "^5.0.15", + "@types/react": "^16.9", "@types/react-sparklines": "^1.7.0", - "@types/zen-observable": "^0.8.0", "classnames": "^2.2.6", "clsx": "^1.1.0", "lodash": "^4.17.15", @@ -61,6 +56,12 @@ "@testing-library/jest-dom": "^5.7.0", "@testing-library/react": "^9.3.2", "@testing-library/user-event": "^10.2.4", + "@types/classnames": "^2.2.9", + "@types/google-protobuf": "^3.7.2", + "@types/jest": "^25.2.2", + "@types/node": "^12.0.0", + "@types/react-helmet": "^5.0.15", + "@types/zen-observable": "^0.8.0", "jest-fetch-mock": "^3.0.3" }, "files": [ diff --git a/packages/dev-utils/package.json b/packages/dev-utils/package.json index 8c4b960ebb..fc77298371 100644 --- a/packages/dev-utils/package.json +++ b/packages/dev-utils/package.json @@ -37,14 +37,17 @@ "@testing-library/jest-dom": "^5.7.0", "@testing-library/react": "^9.3.2", "@testing-library/user-event": "^10.2.4", - "@types/jest": "^25.2.2", - "@types/node": "^12.0.0", + "@types/react": "^16.9", "react": "^16.12.0", "react-dom": "^16.12.0", "react-hot-loader": "^4.12.21", "react-router": "^5.2.0", "react-router-dom": "^5.2.0" }, + "devDependencies": { + "@types/jest": "^25.2.2", + "@types/node": "^12.0.0" + }, "files": [ "dist/**/*.{js,d.ts}" ] diff --git a/packages/test-utils-core/package.json b/packages/test-utils-core/package.json index ad4bbc95c9..f629877a65 100644 --- a/packages/test-utils-core/package.json +++ b/packages/test-utils-core/package.json @@ -30,11 +30,14 @@ "dependencies": { "@testing-library/jest-dom": "^5.7.0", "@testing-library/react": "^9.3.2", - "@types/jest": "^25.2.2", - "@types/node": "^12.0.0", + "@types/react": "^16.9", "react": "^16.12.0", "react-dom": "^16.12.0" }, + "devDependencies": { + "@types/jest": "^25.2.2", + "@types/node": "^12.0.0" + }, "files": [ "dist/**/*.{js,d.ts}" ] diff --git a/packages/test-utils/package.json b/packages/test-utils/package.json index c35e3a622e..1f7afc9bbb 100644 --- a/packages/test-utils/package.json +++ b/packages/test-utils/package.json @@ -36,13 +36,16 @@ "@testing-library/jest-dom": "^5.7.0", "@testing-library/react": "^9.3.2", "@testing-library/user-event": "^10.2.4", - "@types/jest": "^25.2.2", - "@types/node": "^12.0.0", + "@types/react": "^16.9", "react": "^16.12.0", "react-dom": "^16.12.0", "react-router": "^5.2.0", "react-router-dom": "^5.2.0" }, + "devDependencies": { + "@types/jest": "^25.2.2", + "@types/node": "^12.0.0" + }, "files": [ "dist/**/*.{js,d.ts}" ] diff --git a/plugins/tech-radar/package.json b/plugins/tech-radar/package.json index 3a9c75a3f7..22568f8b97 100644 --- a/plugins/tech-radar/package.json +++ b/plugins/tech-radar/package.json @@ -26,6 +26,7 @@ "@material-ui/core": "^4.9.1", "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.45", + "@types/react": "^16.9", "color": "^3.1.2", "d3-force": "^2.0.1", "prop-types": "^15.7.2", diff --git a/scripts/.eslintrc.js b/scripts/.eslintrc.js new file mode 100644 index 0000000000..106fa4246a --- /dev/null +++ b/scripts/.eslintrc.js @@ -0,0 +1,6 @@ +module.exports = { + extends: [require.resolve('@backstage/cli/config/eslint')], + rules: { + 'no-console': 0, + }, +}; diff --git a/scripts/check-type-dependencies.js b/scripts/check-type-dependencies.js new file mode 100755 index 0000000000..5b7daa13d2 --- /dev/null +++ b/scripts/check-type-dependencies.js @@ -0,0 +1,194 @@ +#!/usr/bin/env node +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +const fs = require('fs'); +const { resolve: resolvePath } = require('path'); +// Cba polluting root package.json, we'll have this +// eslint-disable-next-line import/no-extraneous-dependencies +const chalk = require('chalk'); + +async function main() { + // This is from lerna, and cba polluting root package.json + // eslint-disable-next-line import/no-extraneous-dependencies + const LernaProject = require('@lerna/project'); + const project = new LernaProject(resolvePath('.')); + const packages = await project.getPackages(); + + let hadErrors = false; + + for (const pkg of packages) { + if (!shouldCheckTypes(pkg)) { + continue; + } + const { errors } = await checkTypes(pkg); + if (errors.length) { + hadErrors = true; + console.error( + `Incorrect type dependencies in ${chalk.yellow(pkg.name)}:`, + ); + for (const error of errors) { + if (error.name === 'WrongDepError') { + console.error( + ` Move from ${chalk.red(error.from)} to ${chalk.green( + error.to, + )}: ${chalk.cyan(error.dep)}`, + ); + } else if (error.name === 'MissingDepError') { + console.error( + ` Missing a type dependency: ${chalk.cyan(error.dep)}`, + ); + } else { + console.error(` Unknown error, ${chalk.red(error)}`); + } + } + } + } + + if (hadErrors) { + console.error(); + console.error( + chalk.red('At least one package had incorrect type dependencies'), + ); + + process.exit(2); + } +} + +function shouldCheckTypes(pkg) { + return !pkg.private && pkg.get('types'); +} + +/** + * Scan index.d.ts for imports and return errors for any dependency that's + * missing or incorrect in package.json + */ +function checkTypes(pkg) { + const typeDecl = fs.readFileSync( + resolvePath(pkg.location, 'dist/index.d.ts'), + 'utf8', + ); + const deps = (typeDecl.match(/from '.*'/g) || []) + .map(match => match.replace(/from '(.*)'/, '$1')) + .filter(n => !n.startsWith('.')); + + const errors = []; + const typeDeps = []; + for (const dep of deps) { + try { + const typeDep = findTypesPackage(dep, pkg); + if (typeDep) { + typeDeps.push(typeDep); + } + } catch (error) { + errors.push(error); + } + } + + errors.push(...findTypeDepErrors(typeDeps, pkg)); + + return { errors }; +} + +/** + * Find the package used for types. This assumes that types are working is a package + * can be resolved, it doesn't do any checking of presence of types inside the dep. + */ +function findTypesPackage(dep, pkg) { + try { + require.resolve(`@types/${dep}/package.json`, { paths: [pkg.location] }); + return `@types/${dep}`; + } catch { + try { + require.resolve(dep, { paths: [pkg.location] }); + return undefined; + } catch { + try { + // Some type-only modules don't have a working main field, so try resolving package.json too + require.resolve(`${dep}/package.json`, { paths: [pkg.location] }); + return undefined; + } catch { + try { + // Finally check if it's just a .d.ts file + require.resolve(`${dep}.d.ts`, { paths: [pkg.location] }); + return undefined; + } catch { + throw mkErr('MissingDepError', `No types for ${dep}`, { dep }); + } + } + } + } +} + +/** + * Figures out what type dependencies are missing, or should be moved between dep types + */ +function findTypeDepErrors(typeDeps, pkg) { + const devDeps = mkTypeDepSet(pkg.get('devDependencies')); + const deps = mkTypeDepSet(pkg.get('dependencies')); + + const errors = []; + for (const typeDep of typeDeps) { + if (!deps.has(typeDep)) { + if (devDeps.has(typeDep)) { + errors.push( + mkErr('WrongDepError', `Should be dep ${typeDep}`, { + dep: typeDep, + from: 'devDependencies', + to: 'dependencies', + }), + ); + } else { + errors.push( + mkErr('MissingDepError', `No types for ${typeDep}`, { + dep: typeDep, + }), + ); + } + } else { + deps.delete(typeDep); + } + } + + for (const dep of deps) { + errors.push( + mkErr('WrongDepError', `Should be dev dep ${dep}`, { + dep, + from: 'dependencies', + to: 'devDependencies', + }), + ); + } + + return errors; +} + +function mkTypeDepSet(deps) { + const typeDeps = Object.keys(deps || {}).filter(n => n.startsWith('@types/')); + return new Set(typeDeps); +} + +function mkErr(name, msg, extra) { + const error = new Error(msg); + error.name = name; + Object.assign(error, extra); + return error; +} + +main().catch(error => { + console.error(error.stack || error); + process.exit(1); +}); diff --git a/yarn.lock b/yarn.lock index b115b0bf9f..80e3916f53 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4417,7 +4417,7 @@ dependencies: "@types/react" "*" -"@types/react@*", "@types/react@^16.8.19": +"@types/react@*", "@types/react@^16.8.19", "@types/react@^16.9": version "16.9.25" resolved "https://registry.npmjs.org/@types/react/-/react-16.9.25.tgz#6ae2159b40138c792058a23c3c04fd3db49e929e" integrity sha512-Dlj2V72cfYLPNscIG3/SMUOzhzj7GK3bpSrfefwt2YT9GLynvLCCZjbhyF6VsT0q0+aRACRX03TDJGb7cA0cqg==