Skip to content

Commit 4917804

Browse files
authored
build: Check for circular deps with madge (#3631)
* build: Check for circular dep with madge Madge (https://www.npmjs.com/package/madge) is a library that can traverse module dependency trees to check for circular dependencies. We can leverage it using `madge --circular /path/to/index.ts` to make sure that our packages don't contain circular deps. On average madge takes around 2-3 sec, so we are not sacrificing a ton of CI time. * fix(node): Remove circular dependency To remove the circular relationship between backend and parsers, we extract the NodeOptions type into a separate types.ts file. * fix(hub): Remove circular dep between hub and interface To remove this, the interface.ts was removed and all of it's types were moved into hub.ts * fix(hub): Remove circular dep from session To remove this circular dep, we extract the SessionFlusher from session.ts into it's own file. * fix(nextjs): Remove circular dependency To remove the circular dependency between index.server.ts and utils/instrumentServer.ts, we rely on importing Sentry func (like startTransaction and captureException) from `@sentry/node` directly. * build(types): Don't run madge on @sentry/types * build: Run circularDepCheck github action
1 parent 997a782 commit 4917804

35 files changed

+674
-243
lines changed

.github/workflows/build.yml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,29 @@ jobs:
139139
- name: Run linter
140140
run: yarn lint
141141

142+
job_circular_dep_check:
143+
name: Circular Dependency Check
144+
needs: job_build
145+
timeout-minutes: 10
146+
runs-on: ubuntu-latest
147+
steps:
148+
- name: Check out current commit (${{ github.sha }})
149+
uses: actions/checkout@v2
150+
- name: Set up Node
151+
uses: actions/setup-node@v1
152+
- name: Check dependency cache
153+
uses: actions/cache@v2
154+
with:
155+
path: ${{ env.CACHED_DEPENDENCY_PATHS }}
156+
key: ${{ needs.job_build.outputs.dependency_cache_key }}
157+
- name: Check build cache
158+
uses: actions/cache@v2
159+
with:
160+
path: ${{ env.CACHED_BUILD_PATHS }}
161+
key: ${{ env.BUILD_CACHE_KEY }}
162+
- name: Run madge
163+
run: yarn circularDepCheck
164+
142165
job_unit_test:
143166
name: Test (Node ${{ matrix.node }})
144167
needs: job_build

package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414
"codecov": "codecov",
1515
"pack:changed": "lerna run pack --since",
1616
"prepublishOnly": "lerna run --stream --concurrency 1 prepublishOnly",
17-
"postpublish": "make publish-docs && lerna run --stream --concurrency 1 postpublish"
17+
"postpublish": "make publish-docs && lerna run --stream --concurrency 1 postpublish",
18+
"circularDepCheck": "lerna run --parallel circularDepCheck"
1819
},
1920
"volta": {
2021
"node": "14.17.0",
@@ -58,6 +59,7 @@
5859
"karma-browserstack-launcher": "^1.5.1",
5960
"karma-firefox-launcher": "^1.1.0",
6061
"lerna": "3.13.4",
62+
"madge": "4.0.2",
6163
"mocha": "^6.1.4",
6264
"npm-run-all": "^4.1.2",
6365
"prettier": "1.19.0",

packages/angular/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@
5252
"fix": "run-s fix:eslint fix:prettier",
5353
"fix:prettier": "prettier --write \"{src,test}/**/*.ts\"",
5454
"fix:eslint": "eslint . --format stylish --fix",
55-
"pack": "npm pack"
55+
"pack": "npm pack",
56+
"circularDepCheck": "madge --circular src/index.ts"
5657
},
5758
"volta": {
5859
"extends": "../../package.json"

packages/browser/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@
8282
"size:check": "run-p size:check:es5 size:check:es6",
8383
"size:check:es5": "cat build/bundle.min.js | gzip -9 | wc -c | awk '{$1=$1/1024; print \"ES5: \",$1,\"kB\";}'",
8484
"size:check:es6": "cat build/bundle.es6.min.js | gzip -9 | wc -c | awk '{$1=$1/1024; print \"ES6: \",$1,\"kB\";}'",
85-
"pack": "npm pack"
85+
"pack": "npm pack",
86+
"circularDepCheck": "madge --circular src/index.ts"
8687
},
8788
"volta": {
8889
"extends": "../../package.json"

packages/core/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@
4848
"test": "jest",
4949
"test:watch": "jest --watch",
5050
"pack": "npm pack",
51-
"version": "node ../../scripts/versionbump.js src/version.ts"
51+
"version": "node ../../scripts/versionbump.js src/version.ts",
52+
"circularDepCheck": "madge --circular src/index.ts"
5253
},
5354
"volta": {
5455
"extends": "../../package.json"

packages/eslint-config-sdk/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@
4040
"link:yarn": "yarn link",
4141
"lint": "prettier --check \"**/*.js\"",
4242
"fix": "prettier --write \"**/*.js\"",
43-
"pack": "npm pack"
43+
"pack": "npm pack",
44+
"circularDepCheck": "madge --circular src/index.js"
4445
},
4546
"volta": {
4647
"extends": "../../package.json"

packages/eslint-plugin-sdk/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@
3131
"lint": "prettier --check \"{src,test}/**/*.js\"",
3232
"fix": "prettier --write \"{src,test}/**/*.js\"",
3333
"test": "mocha test --recursive",
34-
"pack": "npm pack"
34+
"pack": "npm pack",
35+
"circularDepCheck": "madge --circular src/index.js"
3536
},
3637
"volta": {
3738
"extends": "../../package.json"

packages/gatsby/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@
5959
"fix:eslint": "eslint . --format stylish --fix",
6060
"test": "jest",
6161
"test:watch": "jest --watch",
62-
"pack": "npm pack"
62+
"pack": "npm pack",
63+
"circularDepCheck": "madge --circular src/index.ts"
6364
},
6465
"volta": {
6566
"extends": "../../package.json"

packages/hub/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@
4545
"fix:eslint": "eslint . --format stylish --fix",
4646
"test": "jest",
4747
"test:watch": "jest --watch",
48-
"pack": "npm pack"
48+
"pack": "npm pack",
49+
"circularDepCheck": "madge --circular src/index.ts"
4950
},
5051
"volta": {
5152
"extends": "../../package.json"

packages/hub/src/hub.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import {
2323
} from '@sentry/types';
2424
import { consoleSandbox, dateTimestampInSeconds, getGlobalObject, isNodeEnv, logger, uuid4 } from '@sentry/utils';
2525

26-
import { Carrier, DomainAsCarrier, Layer } from './interfaces';
2726
import { Scope } from './scope';
2827
import { Session } from './session';
2928

@@ -49,6 +48,47 @@ const DEFAULT_BREADCRUMBS = 100;
4948
*/
5049
const MAX_BREADCRUMBS = 100;
5150

51+
/**
52+
* A layer in the process stack.
53+
* @hidden
54+
*/
55+
export interface Layer {
56+
client?: Client;
57+
scope?: Scope;
58+
}
59+
60+
/**
61+
* An object that contains a hub and maintains a scope stack.
62+
* @hidden
63+
*/
64+
export interface Carrier {
65+
__SENTRY__?: {
66+
hub?: Hub;
67+
/**
68+
* Extra Hub properties injected by various SDKs
69+
*/
70+
integrations?: Integration[];
71+
extensions?: {
72+
/** Hack to prevent bundlers from breaking our usage of the domain package in the cross-platform Hub package */
73+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
74+
domain?: { [key: string]: any };
75+
} & {
76+
/** Extension methods for the hub, which are bound to the current Hub instance */
77+
// eslint-disable-next-line @typescript-eslint/ban-types
78+
[key: string]: Function;
79+
};
80+
};
81+
}
82+
83+
/**
84+
* @hidden
85+
* @deprecated Can be removed once `Hub.getActiveDomain` is removed.
86+
*/
87+
export interface DomainAsCarrier extends Carrier {
88+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
89+
members: { [key: string]: any }[];
90+
}
91+
5292
/**
5393
* @inheritDoc
5494
*/

0 commit comments

Comments
 (0)