refactor: switch to NPM workspaces #687
Conversation
not-my-profile
force-pushed
the
npm-workspaces
branch
6 times, most recently
from
f27dc3d
to
66ab973
Compare
not-my-profile
force-pushed
the
npm-workspaces
branch
18 times, most recently
from
5f2dae5
to
565806f
Compare
not-my-profile
force-pushed
the
npm-workspaces
branch
3 times, most recently
from
08a7d0d
to
752892b
Compare
packages/cli/package.json
Outdated
| @@ -45,7 +44,7 @@ | |||
| ], | |||
| "dependencies": { | |||
| "@axe-core/webdriverjs": "^4.7.3", | |||
| "axe-core": "^4.7.0", | |||
| "axe-core": "~4.7.0", | |||
There was a problem hiding this comment.
Axe-core 4.8.1 was released and I don't want to take it with this conversion, so pinned all axe-core versions to 4.7
There was a problem hiding this comment.
This is a breaking change for customers that are already using 4.8, which is allowed by the old version of this specifier and what a customer would have gotten already if they happen to have installed one of these packages since 4.8 was released.
I agree with not changing the requirement in this PR, and I'd also be fine with trying to avoid a lockfile update that moves our testing to 4.8, but I strongly disagree with converting ^ to ~ in all these packages
| @@ -44,7 +44,7 @@ | |||
| "express": "^4.18.2", | |||
| "mocha": "^10.0.0", | |||
| "nyc": "^15.1.0", | |||
| "puppeteer": "^19.7.3", | |||
| "puppeteer": "19.7.3", | |||
There was a problem hiding this comment.
Starting in Puppeteer 19.11 (ish?) they started adding a log that stated that headlness: new would be needed (which massively clogs up the test run as it logs a huge paragraph every test). However, if you add headless: new to the options it breaks the unloaded iframe test. It seems that whatever is in headless: new fixes the problem with unloaded iframes? Will open a tech debt ticket to investigate.
| @@ -70,7 +70,7 @@ | |||
| "ts-node": "^10.9.1", | |||
| "tsup": "^6.7.0", | |||
| "typescript": "^4.8.4", | |||
| "webdriverio": "^8.8.2" | |||
| "webdriverio": "8.8.2" | |||
There was a problem hiding this comment.
Latest webdriverio breaks our typescript build due to this bug with types webdriverio/webdriverio#11055. Will open a tech debt ticket to fix it.
| @@ -32,8 +32,7 @@ | |||
| "prebuild": "rimraf dist", | |||
| "build": "tsc", | |||
| "test": "mocha --timeout 60000 -r ts-node/register 'src/**/**.test.ts'", | |||
| "coverage": "nyc npm run test", | |||
| "prepare": "npm run build" | |||
There was a problem hiding this comment.
Talked with @michael-siek and this isn't needed. When left in it tries to build the cli before it links packages together, which would cause the build to fail as @axe-core/webdriverjs wasn't available yet.
packages/cli/package.json
Outdated
| @@ -45,7 +44,7 @@ | |||
| ], | |||
| "dependencies": { | |||
| "@axe-core/webdriverjs": "^4.7.3", | |||
| "axe-core": "^4.7.0", | |||
| "axe-core": "~4.7.0", | |||
There was a problem hiding this comment.
This is a breaking change for customers that are already using 4.8, which is allowed by the old version of this specifier and what a customer would have gotten already if they happen to have installed one of these packages since 4.8 was released.
I agree with not changing the requirement in this PR, and I'd also be fine with trying to avoid a lockfile update that moves our testing to 4.8, but I strongly disagree with converting ^ to ~ in all these packages
|
|
dbjorge
force-pushed
the
npm-workspaces
branch
from
d1b4ff1
to
228d9db
Compare
|
force-push was to correct author email address in merge from develop, to hopefully fix CLA bot |
|
Reviewed for security |
|
The
lerna bootstrapcommand is deprecated1and resulted in a very weird CI failure (#685).
This commit switches the repository to instead use
NPM workspaces.2
Fixes #685.