Skip to content

Commit

Permalink
fix: ensure that packages properly specify their dependencies
Browse files Browse the repository at this point in the history
Many packages in the monorepo did not specify all of their dependencies; they
were effectively relying on resolution in the monorepo's root
`node_modules`. In a production release of `embark` and `embark[js]-*` packages
this can lead to broken packages.

To fix the problem currently and to help prevent it from happening again, make
use of the `eslint-plugin-import` package's `import/no-extraneous-dependencies`
and `import/no-unresolved` rules. In the root `tslint.json` set
`"no-implicit-dependencies": true`, wich is the tslint equivalent of
`import/no-extraneous-dependencies`; there is no tslint equivalent for
`import/no-unresolved`, but we will eventually replace tslint with an eslint
configuration that checks both `.js` and `.ts` files.

For `import/no-unresolved` to work in our monorepo setup, in most packages add
an `index.js` that has:

```js
module.exports = require('./dist'); // or './dist/lib' in some cases
```

And point `"main"` in `package.json` to `"./index.js"`. Despite what's
indicated in npm's documentation for `package.json`, it's also necessary to add
`"index.js"` to the `"files"` array.

Make sure that all `.js` files that can and should be linted are in fact
linted. For example, files in `packages/embark/src/cmd/` weren't being linted
and many test suites weren't being linted.

Bump all relevant packages to `eslint@6.8.0`.

Fix all linter errors that arose after these changes.

Implement a `check-yarn-lock` script that's run as part of `"ci:full"` and
`"qa:full"`, and can manually be invoked via `yarn cylock` in the root of the
monorepo. The script exits with error if any specifiers are found in
`yarn.lock` for `embark[js][-*]` and/or `@embarklabs/*` (with a few exceptions,
cf. `scripts/check-yarn-lock.js`).
  • Loading branch information
michaelsbradleyjr committed Feb 25, 2020
1 parent ad19b9b commit 3693ebd
Show file tree
Hide file tree
Showing 232 changed files with 985 additions and 723 deletions.
21 changes: 19 additions & 2 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
"node": true
},
"extends": [
"eslint:recommended"
"eslint:recommended",
"plugin:import/typescript"
],
"parser": "babel-eslint",
"plugins": [
"import",
"react"
],
"rules": {
Expand Down Expand Up @@ -72,6 +74,8 @@
"id-blacklist": "error",
"id-length": "off",
"id-match": "error",
"import/no-extraneous-dependencies": "error",
"import/no-unresolved": [2, { "commonjs": true }],
"indent": "off",
"indent-legacy": "off",
"init-declarations": "off",
Expand Down Expand Up @@ -102,7 +106,6 @@
"no-bitwise": "error",
"no-buffer-constructor": "error",
"no-caller": "error",
"no-catch-shadow": "error",
"no-confusing-arrow": "error",
"no-console": "off",
"no-continue": "off",
Expand Down Expand Up @@ -280,5 +283,19 @@
"error",
"never"
]
},
"settings": {
"import/resolver": {
"node": {
"extensions": [
".d.ts",
".js",
".json",
".jsx",
".ts",
".tsx"
]
}
}
}
}
56 changes: 32 additions & 24 deletions babel.config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
/* global module require */

/*
* dependencies of this config should be specified in `./package.json` relative
* to this config file (which should be in the root of the monorepo);
Expand All @@ -21,20 +19,26 @@ module.exports = (api) => {
],
plugins: [
'babel-plugin-macros',
['@babel/plugin-proposal-decorators', {
legacy: true
}],
[
'@babel/plugin-proposal-decorators', {
legacy: true
}
],
'@babel/plugin-proposal-export-namespace-from',
'@babel/plugin-proposal-export-default-from',
'@babel/plugin-syntax-dynamic-import',
['@babel/plugin-proposal-class-properties', {
loose: true
}],
[
'@babel/plugin-proposal-class-properties', {
loose: true
}
],
'@babel/plugin-proposal-nullish-coalescing-operator',
'@babel/plugin-proposal-optional-chaining',
['@babel/plugin-transform-runtime', {
corejs: 3
}]
[
'@babel/plugin-transform-runtime', {
corejs: 3
}
]
],
presets: [
'@babel/preset-env',
Expand All @@ -48,13 +52,15 @@ module.exports = (api) => {

const browser = cloneDeep(base);
browser.plugins[browser.plugins.length - 1][1].useESModules = true;
browser.presets[0] = [browser.presets[0], {
corejs: 3,
modules: false,
shippedProposals: true,
targets: {browsers: ['last 1 version', 'not dead', '> 0.2%']},
useBuiltIns: 'usage'
}];
browser.presets[0] = [
browser.presets[0], {
corejs: 3,
modules: false,
shippedProposals: true,
targets: {browsers: ['last 1 version', 'not dead', '> 0.2%']},
useBuiltIns: 'usage'
}
];

if (env === 'browser' || env.startsWith('browser:')) {
return browser;
Expand All @@ -66,12 +72,14 @@ module.exports = (api) => {
0,
'babel-plugin-dynamic-import-node'
);
node.presets[0] = [node.presets[0], {
corejs: 3,
shippedProposals: true,
targets: {node: '10.17.0'},
useBuiltIns: 'usage'
}];
node.presets[0] = [
node.presets[0], {
corejs: 3,
shippedProposals: true,
targets: {node: '10.17.0'},
useBuiltIns: 'usage'
}
];

if (env === 'node' || env.startsWith('node:')) {
return node;
Expand Down
9 changes: 8 additions & 1 deletion dapps/tests/service/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,22 @@
"haml": "0.4.3"
},
"devDependencies": {
"eslint": "6.8.0",
"rimraf": "3.0.0"
},
"license": "MIT",
"main": "index.js",
"main": "./index.js",
"name": "embark-dapp-test-service",
"private": true,
"scripts": {
"ci": "npm run qa",
"clean": "npm run reset",
"lint": "eslint *.js",
"qa": "npm-run-all lint",
"reset": "npx rimraf embark-*.tgz package"
},
"eslintConfig": {
"extends": "../../../.eslintrc.json"
},
"version": "5.0.0"
}
19 changes: 14 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@
"@babel/plugin-transform-runtime": "7.7.4",
"@babel/preset-env": "7.7.4",
"@babel/preset-typescript": "7.7.4",
"@typescript-eslint/parser": "2.20.0",
"babel-eslint": "10.0.3",
"babel-plugin-dynamic-import-node": "2.3.0",
"babel-plugin-macros": "2.7.1",
"chalk": "2.4.2",
"coveralls": "3.0.9",
"eslint": "6.2.2",
"eslint": "6.8.0",
"eslint-plugin-import": "2.18.2",
"find-up": "4.1.0",
"form-data": "2.5.1",
"fs-extra": "8.1.0",
Expand All @@ -27,7 +29,8 @@
"npm-run-all": "4.1.5",
"nyc": "13.1.0",
"rimraf": "3.0.0",
"semver": "5.6.0"
"semver": "5.6.0",
"typescript": "3.7.2"
},
"engines": {
"node": ">=10.17.0",
Expand All @@ -42,7 +45,7 @@
"build:no-ui": "npm run build -- --ignore embark-ui",
"ci": "node scripts/monorun --ignore embark-dapp-* --stream ci",
"ci:dapps": "lerna run --concurrency=1 --scope embark-dapp-* --stream ci",
"ci:full": "npm-run-all cwtree typecheck \"lint -- --concurrency={1}\" build:no-ui \"test -- --concurrency={1}\" ci:dapps cwtree -- 1",
"ci:full": "npm-run-all cylock cwtree typecheck lint build:no-ui \"test -- --concurrency={1}\" ci:dapps cwtree -- 1",
"clean": "node scripts/monorun --stream clean",
"clean:full": "npx npm-run-all clean clean:top",
"clean:top": "npm run reset:top && npx rimraf node_modules",
Expand All @@ -52,9 +55,12 @@
"coverage:report": "node scripts/coverage-report",
"coveralls": "npm-run-all coverage:collect coverage:coveralls",
"cwtree": "node scripts/check-working-tree",
"cylock": "node scripts/check-yarn-lock",
"deploy:site": "node site/deploy-site",
"globalize": "node scripts/globalize",
"lint": "node scripts/monorun --parallel lint",
"lint": "npm-run-all lint:*",
"lint:packages": "node scripts/monorun --parallel lint",
"lint:top": "eslint *.js scripts/",
"package": "lerna exec --concurrency=1 --no-private --stream -- npm pack",
"postclean": "npx lerna clean --yes",
"postreboot": "yarn install",
Expand All @@ -63,7 +69,7 @@
"preqa:full": "yarn install",
"qa": "node scripts/monorun --ignore embark-dapp-* --stream qa",
"qa:dapps": "lerna run --concurrency=1 --scope embark-dapp-* --stream qa",
"qa:full": "npm-run-all cwtree reboot:full cwtree typecheck \"lint -- --concurrency={1}\" build \"test -- --concurrency={1}\" qa:dapps cwtree -- 1",
"qa:full": "npm-run-all cylock cwtree reboot:full cwtree typecheck lint build \"test -- --concurrency={1}\" qa:dapps cwtree -- 1",
"reboot": "npm run clean",
"reboot:full": "npm run clean:full",
"release": "node scripts/release",
Expand All @@ -84,6 +90,9 @@
"watch:test": "node scripts/monorun --ignore embark-dapp-* --parallel watch:test",
"watch:typecheck": "node scripts/monorun --parallel watch:typecheck"
},
"eslintConfig": {
"extends": "./.eslintrc.json"
},
"workspaces": {
"packages": [
"dapps/templates/*",
Expand Down
1 change: 1 addition & 0 deletions packages/cockpit/api-client/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = require('./dist');
7 changes: 4 additions & 3 deletions packages/cockpit/api-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@
"type": "git",
"url": "https://github.com/embarklabs/embark.git"
},
"main": "./dist/index.js",
"main": "./index.js",
"types": "./dist/index.d.ts",
"files": [
"dist"
"dist",
"index.js"
],
"embark-collective": {
"build:node": true,
Expand All @@ -51,7 +52,7 @@
},
"devDependencies": {
"embark-solo": "^5.1.1",
"eslint": "5.7.0",
"eslint": "6.8.0",
"npm-run-all": "4.1.5",
"rimraf": "3.0.0"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/cockpit/ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"build-storybook": "build-storybook -s public",
"ci": "npm-run-all lint test",
"clean": "npm run reset",
"lint": "eslint src/",
"lint": "eslint scripts/ src/",
"prebuild": "node scripts/copy-monaco-to-public",
"prestart": "node scripts/copy-monaco-to-public dev",
"qa": "npm-run-all lint test build",
Expand Down
2 changes: 0 additions & 2 deletions packages/cockpit/ui/scripts/copy-monaco-to-public.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
/* global __dirname process require */

const findUp = require('find-up');
const {copy, ensureDir} = require('fs-extra');
const path = require('path');
Expand Down
1 change: 1 addition & 0 deletions packages/core/code-runner/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = require('./dist');
18 changes: 13 additions & 5 deletions packages/core/code-runner/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,16 @@
"solidity"
],
"files": [
"dist"
"dist",
"index.js"
],
"license": "MIT",
"repository": {
"directory": "packages/core/code-runner",
"type": "git",
"url": "https://github.com/embarklabs/embark.git"
},
"main": "./dist/index.js",
"main": "./index.js",
"types": "./dist/index.d.ts",
"embark-collective": {
"build:node": true,
Expand All @@ -36,15 +37,19 @@
"ci": "npm run qa",
"clean": "npm run reset",
"lint": "npm-run-all lint:*",
"lint:js": "eslint src/",
"lint:js": "eslint src/ test/",
"lint:ts": "tslint -c tslint.json \"src/**/*.ts\"",
"qa": "npm-run-all lint _typecheck _build test",
"reset": "npx rimraf dist embark-*.tgz package",
"solo": "embark-solo",
"test": "jest"
},
"eslintConfig": {
"extends": "../../../.eslintrc.json"
"extends": [
"../../../.eslintrc.json",
"plugin:jest/recommended",
"plugin:jest/style"
]
},
"dependencies": {
"@babel/runtime-corejs3": "7.7.7",
Expand All @@ -53,6 +58,7 @@
"colors": "1.4.0",
"core-js": "3.6.2",
"embark-core": "^5.2.2",
"embark-i18n": "^5.1.1",
"embark-logger": "^5.2.0",
"embark-utils": "^5.2.0",
"embarkjs": "^5.2.3-nightly.0",
Expand All @@ -66,10 +72,12 @@
"babel-jest": "24.9.0",
"embark-solo": "^5.1.1",
"embark-testing": "^5.2.0",
"eslint": "5.7.0",
"eslint": "6.8.0",
"eslint-plugin-jest": "22.5.1",
"jest": "24.9.0",
"npm-run-all": "4.1.5",
"rimraf": "3.0.0",
"sinon": "7.4.2",
"tslint": "5.20.1",
"typescript": "3.7.2"
},
Expand Down
1 change: 0 additions & 1 deletion packages/core/code-runner/src/fs.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* global module process require */
const fs = require('fs-extra');
const os = require('os');
const parseJson = require('parse-json');
Expand Down
7 changes: 3 additions & 4 deletions packages/core/code-runner/test/code-runner.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ process.env.DAPP_PATH = 'something';

describe('core/code-runner', () => {

const { embark, plugins } = fakeEmbark();
const { embark } = fakeEmbark();

let codeRunner, doneCb;
// eslint-disable-next-line no-unused-vars
let codeRunner;

beforeEach(() => {
codeRunner = new CodeRunner(embark);
doneCb = sinon.fake();
});

afterEach(() => {
Expand All @@ -42,4 +42,3 @@ describe('core/code-runner', () => {
await embark.events.request2('runcode:eval', `testVar.foo = 'bar';`);
});
});

3 changes: 3 additions & 0 deletions packages/core/code-runner/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
{
"path": "../core"
},
{
"path": "../i18n"
},
{
"path": "../logger"
},
Expand Down
1 change: 1 addition & 0 deletions packages/core/console/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = require('./dist');
Loading

0 comments on commit 3693ebd

Please sign in to comment.