TS Test Conversion - #222
Open
Michiel-VandeVelde wants to merge 25 commits into
Open
Michiel-VandeVelde wants to merge 25 commits into
Michiel-VandeVelde wants to merge 25 commits into
Conversation
jitsedesmet
requested changes
Sep 3, 2026
jitsedesmet
left a comment
jitsedesmet
left a comment
There was a problem hiding this comment.
First round of comments/ opinions :)
…op InstanceType - Fold typecheck into postinstall, drop separate CI step (per review) - Migrate controller tests off real ports to light-my-request injection, keep listen() for LinkedDataFragmentsServer-test.ts (relies on response 'error' events as a recoverable signal, incompatible with light-my-request) - Make DummyServer a class exposing next/error directly, drop SpiedController/Partial workaround - Replace InstanceType<typeof X> & Partial<SpiedController> patterns with direct class imports across controller test files - Remove unused supertest/@types/supertest dependency
…ping, narrow HttpResponse
- Add Router interface to core/lib/types.ts, have DatasourceRouter, PageRouter
and QuadPatternRouter implement it explicitly instead of each router-consuming
file declaring its own local duck-typed interface
- Widen RouterRequest.url.pathname to string | null, matching Node's own
url.parse()/UrlObject convention, removing the need for callers to coerce
null to undefined
- Apply reviewer's suggested { url: parsed } simplification in test-helpers.ts,
now unblocked by the pathname widening above
- Make test-helpers.ts's HttpResponse extend IncomingMessage instead of
Readable, mirroring the existing StreamCapture/ServerResponse pattern
Adopts the TypeScript layout used in Comunica: a single tsconfig.json that
builds the shipped code (yarn build is bare tsc, run from postinstall), plus
tsconfig.eslint.json, which exists only so typescript-eslint's type-aware
rules have a program containing the test files and the Vitest config. The
build/typecheck split (tsconfig.build.json, tsconfig.typecheck.json) and the
typecheck script are gone. "module" stays "commonjs", as this monorepo is
CommonJS throughout: no package.json in it declares "type": "module".
skipLibCheck stays off, which is what forced most of the dependency work:
- Under TypeScript 5, "module": "commonjs" implied node10 resolution, which
cannot follow package.json "exports", so Vitest's own type definitions
failed to resolve their subpath imports; that is what the deleted
tsconfig.typecheck.json worked around. TypeScript 6 resolves "commonjs"
through the "exports" map, so a single module setting now serves both the
build and the test tooling. TypeScript 6 also deprecates
esModuleInterop=false, so interop is now on; the emitted requires gain the
usual __importStar helpers.
- The Vitest config no longer computes a root directory from import.meta.url,
which a CommonJS program cannot type-check. It resolves compiled output back
to its sources through a Vite plugin instead, as Comunica does. It keeps its
.mts extension so that Vite still loads it as a module, rather than as the
CommonJS its ESM syntax would warn about.
- immutable@3.8.2, reached through componentsjs 4 -> rdf-parse 1 ->
@comunica/core 1, declares its namespaces with the old `declare module Foo`
syntax that TypeScript 6 rejects. Pinned to ^5 through resolutions.
- The RDF typing layer moves off the deprecated, undeclared @types/rdf-js:
@rdfjs/types ^2 is now an explicit dependency of every package that uses
it, and the rdf-js imports point there. rdf-string, sparqljson-parse,
jsonld-streaming-parser/serializer, rdfa-streaming-parser and rdf-object
are realigned on that same major. rdf-string stays on 1.x on purpose: 2.x
builds language literals through the RDF 1.2 `{ language, direction }` form,
which n3 1.x's data factory does not accept.
- n3 has never shipped type declarations; the types came from a stale lock
entry. @types/n3 is now an explicit devDependency and the n3 ranges are
unified, with n3, rdf-string and asynciterator declared in the packages
that were using them without saying so.
componentsjs stays on ^4. Version 6 type-checks cleanly but rejects our
componentsjs-4-era component definitions at startup (optional parameters
without a default need a ParameterRangeUndefined range), and componentsjs 4
in turn cannot use rdf-parse >= 2. ESLint likewise stays on 7 with
typescript-eslint 5, which still lints cleanly against TypeScript 6;
upgrading to Comunica's versions surfaces ~96 findings that need rule
decisions first.
Verified: zero type errors in both programs, no output emitted next to the
test sources, lint clean, 597 tests passing, and the server still answers a
Turtle-datasource config with byte-identical TriG.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # package.json # yarn.lock
jitsedesmet
requested changes
Sep 28, 2026
jitsedesmet
left a comment
jitsedesmet
left a comment
There was a problem hiding this comment.
Good stuff @Michiel-VandeVelde ! :D
…rtedFeatures option, dedupe SPARQL URL assertion - createRequest() takes an optional headers argument instead of requiring callers to mutate request.headers after construction - QuadPatternFragmentsController-test.ts uses Datasource's real supportedFeatureList constructor parameter instead of assigning datasource.supportedFeatures post-construction, which silently bypassed the class's own Object.freeze() - Extract the repeated SPARQL request-URL parse-and-assert pattern in SparqlDatasource-test.ts into a single expectRequestedQuery() helper
jitsedesmet
reviewed
Sep 30, 2026
Comment on lines
+56
to
+59
| // Query has no index signature; this lets the test tables below use an | ||
| // arbitrary 'a' field as a stand-in for "pre-existing data that should survive". | ||
| export type TestQuery = Query & { a?: number }; | ||
| export type QueryParamsTestCase = [string, string, string, TestQuery, TestQuery]; |
There was a problem hiding this comment.
There are no tests here. Maybe we can clean up the tests this came from some more?
Neither these types seem to make any sense here?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.