Skip to content

refactor: have all packages extend tsconfig from root#1297

Merged
straker merged 11 commits into
developfrom
root-tsconfig
Mar 10, 2026
Merged

refactor: have all packages extend tsconfig from root#1297
straker merged 11 commits into
developfrom
root-tsconfig

Conversation

@straker
Copy link
Copy Markdown
Contributor

@straker straker commented Mar 9, 2026

Continuing to clean up the repo a bit and having all tsconfig files extend from the root one. Also expanded the root tsconfig to have more strict options. Doing so revealed a type issue in cli when we tried to analyze the page and get the node style callback back so I changed the type to use a control flow analysis for dependent parameters which resolved the issue. This let me remove the type casts of the parameters and use what the function returned.

No QA required

@straker straker requested a review from a team as a code owner March 9, 2026 20:36
@straker straker requested a review from Garbee March 9, 2026 20:36
@Garbee
Copy link
Copy Markdown
Member

Garbee commented Mar 9, 2026

From the test failures. Looks like

import { version } from '../../package.json';
needs to have with { type: 'json' }; added to the import.

Then the playwright things seem to be failing due to code coverage. I have no idea why the changes here would impact the Playwright tests, unless they overlap with the CLI bit somehow. If the coverage data does show overlap for Playwright, we should address it here. Otherwise I'd lower the threshold slightly. I don't think the 2% is a big problem right now for that area.

@straker
Copy link
Copy Markdown
Contributor Author

straker commented Mar 9, 2026

Ya, what's even weirder is that odd changes to the root tsconfig breaks things completely, like adding "noImplicitReturns": true, makes some of the tests break. I haven't been able to find a reason that would be the case since no implicit returns doesn't affect tests as far as I'm aware

Exception during run: Error: Directory import '/home/runner/work/axe-core-npm/axe-core-npm/packages/webdriverio/src' is not supported resolving ES modules imported from /home/runner/work/axe-core-npm/axe-core-npm/packages/webdriverio/test/axe-webdriverio.spec.ts

@Garbee
Copy link
Copy Markdown
Member

Garbee commented Mar 9, 2026

Very interesting. I'll try to check things out in the morning based on wherever you end up tonight. I'm too tired after my day to have any more useful brain energy right now.

chutchins25
chutchins25 previously approved these changes Mar 10, 2026
Copy link
Copy Markdown

@chutchins25 chutchins25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Comment thread tsconfig.json
Comment on lines +3 to +19
"allowJs": true,
"declaration": true,
"esModuleInterop": true,
"lib": ["dom", "esnext"],
"strict": true,
"module": "esnext",
"moduleResolution": "node",
"noImplicitAny": true,
"noImplicitThis": true,
"outDir": "dist",
"pretty": true,
"removeComments": true,
"resolveJsonModule": true,
"skipLibCheck": true,
"sourceMap": true,
"declaration": true,
"outDir": "dist",
"esModuleInterop": true,
"resolveJsonModule": true
"strict": true,
"strictNullChecks": true,
"target": "esnext"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for putting these in alphabetical order!

Comment thread packages/cli/package.json Outdated
Copy link
Copy Markdown
Member

@Garbee Garbee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from the last batch of changes and our discussion in slack. Thanks for getting a comment in from that before I could.

@straker straker merged commit 22ddeaf into develop Mar 10, 2026
38 checks passed
@straker straker deleted the root-tsconfig branch March 10, 2026 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants