Ship TypeScript declarations for the public API - #71
Merged
Conversation
The library stays JavaScript. The types are hand written in types/jdate.d.ts and emitted into lib/ by the build, so consumers get them from the package itself -- there is no @types/jalali-date on npm to install, and now no reason for one to appear. Generating them from the JSDoc in src/ with tsc --allowJs was the obvious alternative and does not work here. The comments use @params, which tsc ignores, so every one would have to be rewritten; and the constructor is variadic, so constructor(...args) would emit ...args: any[] and lose the four call forms, which are the most useful thing in the file to type. Hand writing costs a second source of truth, kept honest by the checks below. The subtle part is that one declaration cannot describe both bundles. lib/jdate.cjs ends with module.exports = module.exports.default, so require() hands back the class itself and its declaration has to say `export = JDate`; lib/jdate.mjs exports the class as default and needs `export default`. Sharing a single `export default` file between them would type require('jalali-date') as { default: JDate }, which is wrong and silently so. types/jdate.d.ts is therefore a body with no export statement, and the build appends one of two footers to it -- the same way it already appends footers to the JS bundles. `export =` forbids every other top-level export, which is why the CJS side re-exports the type names through a namespace merged into the class. lib/index.d.ts is a third copy carrying the CJS footer, for the top-level "types" field to point at. It looks redundant beside jdate.d.cts and is not: TypeScript did not learn the .d.cts extension until 4.7, so aiming "types" straight at that file leaves every older version with no types at all. 4.6 reports "Could not find a declaration file for module 'jalali-date'". Two checks guard this, both in CI after the build. test:types compiles tests/types against the shipped declarations under the two resolution modes in common use: nodenext, which tells the .mts and .cts fixtures apart, and bundler. test:exports runs attw over the packed tarball. They cover different failures and neither is redundant -- attw checks that every entry point resolves to types and that no format masquerades as another, but it does not look at what those types export. Swapping the CJS footer for the ESM one leaves attw fully green while the .cts fixture fails at once with "This expression is not constructable". That was verified by doing it. The constructor is typed as three overloads covering the four call forms. JalaliDate is a three-element tuple rather than number[] deliberately: new JDate([1396, 8]) throws nothing at runtime, it quietly builds an Invalid Date, so the arity is the only place that mistake can be caught. Config entry counts stay a runtime check instead, because a name list built elsewhere infers as string[] rather than as a tuple and should still be passable -- the README's own example is that shape. Reviewing the declarations against the runtime turned up one defect in them. The config-only overload had been given `config?: JDateConfig | null`, copied from the other two. A trailing null is dropped only when a whole date is left standing beside it, so new JDate(null) reaches no branch and throws "Unexpected input", and new JDate(maybeConfig) on a nullable variable would have compiled clean and thrown in production. The union is gone from that overload and a fixture pins it. new JDate(undefined) throws for the same reason and cannot be typed out of reach, since TypeScript lets undefined through any optional parameter; the declaration says so rather than leaving it to be discovered. Verified from the consumer's side and not only in place: the tarball was packed, installed into a scratch project and type-checked as a real dependency under TypeScript 6 (nodenext ESM, nodenext CJS, bundler), 5.9 (nodenext and legacy node) and 4.6 (legacy node). The runtime agrees with the declarations -- require() yields function JDate with no .default, and the ESM default import yields the class. No runtime code changed. The 101 tests and the four JS bundles are byte for byte what they were.
arashm
added a commit
that referenced
this pull request
Jul 28, 2026
Three additions since 1.4.0, all backwards compatible in the ordinary case, so this is a minor bump. The three display-name lists are now configurable (#69). `monthNames`, `dayNames` and `abbrDays` were read straight out of constants.js, so rendering anything but Persian meant forking the library. They are defaults behind a validated config object, settable app-wide with `setDefaultConfig()` or per instance as the last constructor argument. `persianNumerical` makes the numeric identifiers render Persian digits (#70). It is false by default, because flipping it would silently change formatted output for every existing caller. Scope is the seven numeric tokens only — name identifiers print verbatim out of the config and bracketed literals are untouched. TypeScript declarations ship with the package (#71). The library stays JavaScript; the declarations are hand written in types/jdate.d.ts and the build emits one per bundle, because `require()` returns the class itself and needs `export =` where the ESM bundle needs `export default`. One narrowing, deliberate: a trailing argument that is neither nullish nor a plain object now throws `Unexpected input` rather than being silently ignored. Now that the position means something, dropping a misshapen config quietly would hide the likely mistake of passing the name array directly. Two bugs fixed along the way. Date detection went through `instanceof`, which is per-realm, so a Date from an iframe or vm threw `Unexpected input`; all three checks now read the internal slot via `Object.prototype.toString`. And making the last argument meaningful had made a trailing null or undefined an error, which is the ordinary shape of an optional argument — it is dropped again when a whole date form remains beside it. 101 tests pass, lint is clean and npm audit reports 0 vulnerabilities. The declarations are checked from both sides: tests/types compiles fixtures against them under nodenext and bundler, attw checks the packed tarball, and tests/bundles.mjs asserts the built bundles still have the export shapes the declarations claim (#72). The tarball was installed into a scratch project and exercised through require, import and tsc 6.0.3 before tagging.
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
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.
Makes the package usable from TypeScript without converting it to TypeScript. The library stays JavaScript; the declarations are hand written in
types/jdate.d.tsand emitted intolib/by the build.Why not generate them from the JSDoc
tsc --allowJs --emitDeclarationOnlywas the obvious route and does not work here. The comments insrc/use@params, which tsc ignores, so every one would need rewriting; and the constructor is variadic, soconstructor(...args)emits...args: any[]and loses the four call forms — the most useful thing in the file to type. Hand writing costs a second source of truth, which is what the two checks below are for.One declaration cannot describe both bundles
lib/jdate.cjsends withmodule.exports = module.exports.default, sorequire()returns the class itself and its declaration must sayexport = JDate.lib/jdate.mjsexports the class asdefaultand needsexport default. A single sharedexport defaultfile would typerequire('jalali-date')as{ default: JDate }— wrong, and silently so.So
types/jdate.d.tsis a body with no export statement, and the build appends one of two footers, the same way it already appends footers to the JS bundles:lib/jdate.d.mtsexport default JDatelib/jdate.mjslib/jdate.d.ctsexport = JDatelib/jdate.cjslib/index.d.tsexport = JDate"types"fieldexport =forbids any other top-level export, so the CJS side re-exports the type names through a namespace merged into the class (JDate.JDateConfig).lib/index.d.tslooks redundant besidejdate.d.ctsand is not: TypeScript did not learn the.d.ctsextension until 4.7, so pointing"types"straight at that file leaves every older version with no types at all — 4.6 reportsCould not find a declaration file for module 'jalali-date'.Two checks, both in CI after the build
npm run test:types— compilestests/types/against the shipped declarations under both resolution modes in common use:nodenext(which tells the.mtsand.ctsfixtures apart) andbundler.npm run test:exports— runsattwover the packed tarball.Neither is redundant. attw checks that every entry point resolves to types and that no format masquerades as another, but it does not look at what those types export: swapping the CJS footer for the ESM one leaves attw fully green, while the
.ctsfixture fails immediately withThis expression is not constructable. Verified by doing it.API notes
The constructor is three overloads covering the four call forms.
JalaliDateis a three-element tuple rather thannumber[]on purpose —new JDate([1396, 8])throws nothing at runtime, it quietly builds an Invalid Date, so the arity is the only place that mistake can be caught. Config entry counts stay a runtime check, because a name list built elsewhere infers asstring[]rather than as a tuple and should still be passable; the README's own example is that shape.Reviewing the declarations against the runtime found one defect in them: the config-only overload had been given
config?: JDateConfig | null, copied from the other two. A trailing null is dropped only when a whole date is left standing beside it, sonew JDate(null)reaches no branch and throwsUnexpected input— andnew JDate(maybeConfig)on a nullable variable would have compiled clean and thrown in production. Fixed, with a fixture pinning it.Verification
Packed the tarball, installed it into a scratch project and type-checked it as a real dependency: TS 6 (nodenext ESM, nodenext CJS, bundler), TS 5.9 (nodenext, legacy
node), TS 4.6 (legacynode) — all clean. Runtime agrees with the declarations:require()yieldsfunction JDatewith no.default, the ESM default import yields the class.A fresh
npm run buildleaves the tree clean, so the checked-inlib/declarations are exactly reproducible from source.No runtime code changed. The 101 tests and the four JS bundles are byte for byte what they were.
Not included
Nothing asserts that the built bundles still have the export shapes the declarations claim — if a build footer changed, types and reality would drift silently. A smoke test importing
lib/jdate.cjsandlib/jdate.mjswould close that, but it makesnpm testdepend on a prior build, so it is left out of this change.