feat: tighten TypeScript type declarations — replace any with concrete types#669
feat: tighten TypeScript type declarations — replace any with concrete types#669mttrbrts wants to merge 6 commits into
any with concrete types#669Conversation
Coverage Report for CI Build 25613024897Coverage remained the same at 72.194%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
da7fa0c to
81df28e
Compare
…ete types Update JSDoc @param and @returns annotations on the priority public API methods across four packages so that generated .d.ts files no longer use `any` where a more specific type is known: - CommonMarkTransformer: toMarkdown, removeFormatting (object input), toTokens (object[] return), fromTokens (object[] param, object return), getSerializer (Serializer return) - CiceroMarkTransformer: getClauseText, fromCiceroEdit, fromCommonMark, toMarkdown, toCommonMark, toCiceroMarkUnwrapped (object I/O), toTokens/fromTokens (object[]/object), getSerializer (Serializer) - HtmlTransformer: toHtml (object input), toCiceroMark (object return) - TemplateMarkTransformer: getSerializer (object return) Regenerated .d.ts files in each package with `tsc`. Closes #668 Signed-off-by: Matt Roberts <matt@rbrts.uk> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Matt Roberts <code@rbrts.uk>
- ToMarkdownVisitor.toMarkdown: {*} input → {object}
- TransformEngine.transformToDestination/transform: source {*} → {object|string},
returns {*}/{Promise} → {Promise<object|string>}
- TransformEngine.registerTransformation: transform {*} → {Function}
- TransformEngine.registerExtension: extension {*} → {object}
- transform.js module exports: add JSDoc to formatDescriptor, transform,
generateTransformationDiagram, and builtinEngine so tsc can emit
typed declarations rather than any
Regenerated .d.ts files in markdown-common and markdown-transform.
Signed-off-by: Matt Roberts <matt@rbrts.uk>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Matt Roberts <code@rbrts.uk>
…schemas Add a codegen script that runs TypescriptVisitor over all four CTO models (CommonMark, CiceroMark, ConcertoMeta, TemplateMark) and writes clean .d.ts files to types/model/. The build:types script now runs codegen before tsc so model types are always up to date. Update JSDoc @typedef annotations in CommonMarkTransformer and CiceroMarkTransformer to reference the generated interfaces (IDocument, INode, IClause) via package-qualified paths, so tsc emits non-relative imports that resolve correctly from any consumer package. Result: all public API methods now carry specific model types in their generated .d.ts signatures rather than plain object. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Matt Roberts <code@rbrts.uk>
…ger in codegen script strict mode is the default in concerto-core v4. Signed-off-by: Matt Roberts <code@rbrts.uk> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Matt Roberts <code@rbrts.uk>
23736be to
54c752c
Compare
There was a problem hiding this comment.
Pull request overview
This PR tightens TypeScript declarations across the Accord Project Markdown Transform monorepo by introducing generated Concerto model .d.ts interfaces in @accordproject/markdown-common and updating public transformer/engine APIs to use more concrete types instead of any/object.
Changes:
- Add a
markdown-commoncodegen script to generate and publish Concerto model type declarations undertypes/model/*. - Update transformer and transform-engine JSDoc/types so emitted
.d.tssignatures use more specific model types (e.g.,IDocument,INode) and clearer input/return types. - Regenerate downstream package declaration files (
markdown-cicero,markdown-template,markdown-html,markdown-transform) to reference the published model types.
Reviewed changes
Copilot reviewed 12 out of 28 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/markdown-transform/types/lib/transformEngine.d.ts | Updates TransformEngine method signatures in emitted declarations. |
| packages/markdown-transform/types/lib/transform.d.ts | Tightens top-level API declaration signatures for transform, formatDescriptor, etc. |
| packages/markdown-transform/types/lib/builtinTransforms.d.ts | Regenerates builtin transform graph typings to return model types in some paths. |
| packages/markdown-transform/types/index.d.ts | Updates package surface typings and builtin transformation graph typings. |
| packages/markdown-transform/src/transformEngine.js | Updates JSDoc for TransformEngine inputs/outputs and registration APIs. |
| packages/markdown-transform/src/transform.js | Adds/updates JSDoc for exported helpers to improve emitted types. |
| packages/markdown-transform/lib/transformEngine.js | Built output mirroring src/transformEngine.js JSDoc updates. |
| packages/markdown-transform/lib/transform.js | Built output mirroring src/transform.js JSDoc updates. |
| packages/markdown-template/types/lib/TemplateMarkTransformer.d.ts | Tightens emitted declaration for TemplateMarkTransformer serializer accessor. |
| packages/markdown-template/src/TemplateMarkTransformer.js | Updates JSDoc for TemplateMarkTransformer serializer accessor. |
| packages/markdown-template/lib/TemplateMarkTransformer.js | Built output mirroring TemplateMarkTransformer JSDoc updates. |
| packages/markdown-html/types/lib/HtmlTransformer.d.ts | Tightens HtmlTransformer declarations (toHtml, toCiceroMark). |
| packages/markdown-html/src/HtmlTransformer.js | Updates JSDoc types for HtmlTransformer inputs/outputs. |
| packages/markdown-common/types/model/templatemark.d.ts | New generated model declaration file for TemplateMark namespace. |
| packages/markdown-common/types/model/index.d.ts | Barrel export for generated model type declarations. |
| packages/markdown-common/types/model/concerto-metamodel.d.ts | New generated model declaration file for Concerto metamodel types. |
| packages/markdown-common/types/model/concerto-base.d.ts | New generated model declaration file for Concerto base types. |
| packages/markdown-common/types/model/commonmark.d.ts | New generated model declaration file for CommonMark DOM interfaces. |
| packages/markdown-common/types/model/ciceromark.d.ts | New generated model declaration file for CiceroMark interfaces. |
| packages/markdown-common/types/lib/ToMarkdownVisitor.d.ts | Tightens emitted declaration for ToMarkdownVisitor input type. |
| packages/markdown-common/types/lib/CommonMarkTransformer.d.ts | Tightens CommonMarkTransformer public API to use model interfaces. |
| packages/markdown-common/scripts/generate-model-types.js | Adds codegen script to generate types/model/*.d.ts from CTO models. |
| packages/markdown-common/package.json | Adds build:model-types and runs it before tsc; adds concerto-codegen devDep. |
| packages/markdown-common/lib/ToMarkdownVisitor.js | Built output reflecting updated JSDoc for ToMarkdownVisitor. |
| packages/markdown-common/lib/CommonMarkTransformer.js | Adds typedef imports + tightens JSDoc return/param types. |
| packages/markdown-cicero/types/lib/CiceroMarkTransformer.d.ts | Tightens CiceroMarkTransformer public API to use model interfaces. |
| packages/markdown-cicero/lib/CiceroMarkTransformer.js | Adds typedef imports + tightens JSDoc types for CiceroMarkTransformer. |
| package-lock.json | Locks newly added dev dependency (@accordproject/concerto-codegen) and transitive deps. |
Comments suppressed due to low confidence (1)
packages/markdown-template/src/TemplateMarkTransformer.js:102
getSerializer()returnstemplateMarkManager.serializer, which is constructed as aSerializerfrom@accordproject/concerto-core(seetemplatemarkutil.js). Returning/documenting it as a plainobjectloses useful type information and is inconsistent with the other transformers’getSerializer(): Serializer. UseSerializeras the return type in JSDoc so.d.tsgeneration stays accurate.
/**
* Get TemplateMark serializer
* @return {object} templatemark serializer
*/
getSerializer() {
return templateMarkManager.serializer;
}
| * @param {object|string} source the input for the transformation | ||
| * @param {string} sourceFormat the input format | ||
| * @param {string[]} destinationFormat the destination format as an array | ||
| * @param {object} parameters the transform parameters |
| * @param {object|string} source the input for the transformation | ||
| * @param {string} sourceFormat the input format | ||
| * @param {string} destinationFormat the destination format | ||
| * @param {object} parameters the transform parameters |
| * @param {string} sourceFormat the input format | ||
| * @param {string[]} destinationFormat the destination format as an array, | ||
| * the transformation are applied in order to reach all formats in the array | ||
| * @param {object} parameters the transform parameters |
| mm.accept(new CodeGen.TypescriptVisitor(), { | ||
| fileWriter: { | ||
| openFile: (f) => { files[f] = ''; }, | ||
| writeLine: (i, l) => { files[Object.keys(files).at(-1)] += ' '.repeat(i) + l + '\n'; }, | ||
| closeFile: () => {} |
There was a problem hiding this comment.
Instead upgrade the Node version to >=22
There was a problem hiding this comment.
Done — upgraded minimum Node version to >=22 across all package.json files and removed Node 20.x from the CI matrix in commit 8588544.
|
@copilot apply changes based on the comments in this thread. Also update the minimum node version to 22 |
- Mark `parameters` as `[parameters]` (optional) in JSDoc for `transform.js` and `transformEngine.js` (src/ and lib/) so generated `.d.ts` files emit `parameters?: object` instead of `parameters: object` - Update minimum Node version from >=18/>=15 to >=22 in all package.json engines fields - Remove Node 20.x from CI build matrix (minimum is now 22) - Regenerate `types/` declarations to reflect optional parameters Signed-off-by: Copilot <copilot@github.com> Agent-Logs-Url: https://github.com/accordproject/markdown-transform/sessions/b58c7539-b316-4d97-b13c-bde57770d0b7 Co-authored-by: mttrbrts <7544022+mttrbrts@users.noreply.github.com>
Applied in commit
|
|
Hi @mttrbrts, I went through the current
I patched those on top of the current PR head in a follow-up branch so the diff stays narrow. |
|
@Rishabh060105 thanks for that patch, it's looking really strong now. I'll wait for a review from another maintainer before merging. @accordproject/maintainers |
Summary
Closes #668
This PR tightens TypeScript declarations across the public API in two phases:
Phase 1 (previous commits): Replace
anywithobject,object[], andSerializeron priority public API methods.Phase 2 (this commit): Generate TypeScript interfaces directly from the CTO schema files and use them in all public API signatures — replacing the remaining
objecttypes with specific model types.Phase 2 detail
A new codegen script (
packages/markdown-common/scripts/generate-model-types.js) runs@accordproject/concerto-codegen'sTypescriptVisitorover all four CTO models (CommonMark, CiceroMark, ConcertoMeta, TemplateMark) and writes.d.tsfiles topackages/markdown-common/types/model/. Thebuild:typesscript now runs codegen beforetsc.All generated interfaces are published from
@accordproject/markdown-common— other packages reference them via package-qualified paths so generated declarations resolve correctly from any consumer.Before (Phase 1 result):
After (Phase 2 result):
Consumers can import model types directly:
Changes by package
markdown-common: add@accordproject/concerto-codegendevDep; addscripts/generate-model-types.js; addbuild:model-typesscript;@typedefannotations inCommonMarkTransformer.jsforIDocument/INode; newtypes/model/directory with 5 generated interface files + barrelindex.d.tsmarkdown-cicero:@typedefannotations inCiceroMarkTransformer.jsforIDocument,ICiceroDocument,IClause; all public methods use specific typesmarkdown-transform: regeneratedtypes/index.d.tsandtypes/lib/builtinTransforms.d.ts— transformation graph methods now returnIDocumentinstead ofobjectTest plan
npm run build:typespasses cleanly inmarkdown-common,markdown-cicero,markdown-templatetemplate-enginelinked to these packages builds with zero TypeScript errorstypes/model/barrel export verified:import type { IDocument, IClause } from '@accordproject/markdown-common/types/model'npm test --workspacesSigned-off-by: Matt Roberts matt@rbrts.uk
🤖 Generated with Claude Code