Skip to content

add TypeScript noEmit type‑check for ESM integration tests #7981

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: dev-2.0
Choose a base branch
from

Conversation

pratham-radadiya
Copy link

Added integration tests

Resolves #7938

Changes:

  • Vitest type‑check integration: Added vitest.config.ts at the project root, with a typecheck block under the test section that only picks up files matching test/integrations/**/*.test-d.ts.

  • test/tsconfig.test.json: TS compiler options and include globs

  • index.test-d.ts: minimal declare-only sketch to validate p5 types

  • package.json: Added"test:types": "tsc --noEmit -p test/tsconfig.test.json" to run the type‑only suite.

  • Maintain: Stay within Vitest’s ecosystem to avoid tooling drift.

  • Focus: Cover only type correctness (no React/Vue runtimes), per original issue scope.

PR Checklist

@pratham-radadiya
Copy link
Author

hii, @Vaivaswat2244 @error-four-o-four @limzykenneth @ksen0 @davepagurek

i made a new changes in issue #7938, please check it out.

@limzykenneth
Copy link
Member

@pratham-radadiya Sorry but I think I need to be a bit honest here, the changes here don't really make any sense. We run our tests in the browser with Vitest's browser mode which means we don't need jsdom; we already use Vitest and as such has the relevant Vitest configuration in vitest.workspace.mjs, adding a vitest.config.ts means our regular tests will no longer run; adding a vitest run npm script under test:integration is an overlap with the existing test script while being more limited (vitest run is the same as vitest but turns off watch mode).

Judging from the code you submitted for PRs and formats on natural language comments, I feel like you are heavily relying on AI to resolve code problems and write responding comments, these are not likely to end up being useful for completing a relatively complex contributions such as what we have for this issue, nor useful for your own skill's improvements. If you'd like some technical guidance or help, we are available and can point you in specific directions so we would appreciate if you reach out to us for questions instead of an AI if you are unsure about anything.

In the case of this specific issue, if you don't already have a degree of understanding of Vitest and p5.js' Vitest setup, that should be where you start investigating and building an understanding on how these systems work. Alternatively, you can have a look at other less complex issues that may need fixing, put forward in your own words your understanding of the issue, and possibly propose a potential fix. Open source contributions is not about getting code & PRs merged nor speed of creating an implementation, it is a collaborative process that involve discussions with people and working gradually towards a potential solution.

@pratham-radadiya
Copy link
Author

Hii @limzykenneth, Thank you for your honest feedback and for pointing these things out. You’re absolutely right that I need to build up my own understanding of how p5.js uses Vitest in browser mode before proposing changes, and i will make sure every change and explanation I provide comes from my own understanding. can you please share your linkedin profile?

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.

2 participants