Skip to content

chore(types): type check all files including tests#2752

Draft
johncowen wants to merge 1 commit intomainfrom
scratch/check-types
Draft

chore(types): type check all files including tests#2752
johncowen wants to merge 1 commit intomainfrom
scratch/check-types

Conversation

@johncowen
Copy link
Contributor

@johncowen johncowen commented May 14, 2025

We noticed that we could no longer upgrade kongponents due to type issues: kumahq/kuma-gui#3883. The specific issue we are seeing is that we can no longer pass KRadio to app.component i.e.

app.component('KRadio', KRadio)

Whilst we could see there was a type issue, the bigger question is, how come kongponents can publish a release when the types of some components don't match certain Vue native types? Therefore preventing folks from using app.component.

Kongponents itself doesn't use specific app.component('Kadio', KRadio) anywhere, instead it uses import * and a loop, hence the issue doesn't arise in the library itself only for downstream conusmers, but we noticed the issue does also appear in tests for cy.mount(KRadio)

This led us to notice that CI type checking was completely disabled on all cy.ts files here #828, therefore blanket removing an automatic form of unit testing from all tests. Its not immediately clear from the PR description in #828 as to why.

This PR enables type checking on component tests as an example to show the incorrect types that are apparent if you also type check the tests.

There are quite a few errors, so without understanding why type checking was disabled, we've drafted a PR to help show the issue. I'll also make a corresponding issue to go with this PR for someone to respond to/take a look at fixing the type issues.

Once fixed this PR can be closed as its just to provide a reproduction/example.

Signed-off-by: John Cowen <john.cowen@konghq.com>
@netlify
Copy link

netlify bot commented May 14, 2025

Deploy Preview for kongponents-sandbox ready!

Name Link
🔨 Latest commit 4ba32a9
🔍 Latest deploy log https://app.netlify.com/sites/kongponents-sandbox/deploys/68245090bfc507000888a46e
😎 Deploy Preview https://deploy-preview-2752--kongponents-sandbox.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented May 14, 2025

Deploy Preview for kongponents ready!

Name Link
🔨 Latest commit 4ba32a9
🔍 Latest deploy log https://app.netlify.com/sites/kongponents/deploys/68245090ba5956000850c0a2
😎 Deploy Preview https://deploy-preview-2752--kongponents.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

"docs:serve": "vitepress serve docs",
"docs:preview": "vitepress preview docs --port 8080",
"typecheck": "vue-tsc -p './tsconfig.build.json' --noEmit",
"typecheck": "vue-tsc --noEmit",
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't exclude the tsconfig.build.json here, but rather update it to include all types we want to check.

There are some types in the project that likely should be excluded from checking during the build/release process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep I'm fine whichever. I guess whichever option means we get typechecking on .ts files.

Do you have an example of which files likely should be excluded? (that are a different set to those that are in tsconfig.json)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just incase/FYI I wasn't planning on finishing up this one myself/personally it was just to give a reproduction of the issue. I'd rather leave it to the owners of the repo to decide how to proceed seeing as it will need a good bit of fix up in the tests. In my mind as long as .ts files are being run through typescript/being typechecked then its fine by me 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants