Check the bundles still export what the types claim - #72
Merged
Conversation
The previous change shipped declarations describing an export shape for each bundle and left nothing checking that the bundles still had it. tests/types compiles against the declarations, so it sees only what they claim. attw checks that the entry points resolve to types, not what those types export -- swapping the CJS footer for the ESM one leaves it fully green. The unit suite imports src/ and never loads lib/ at all. A build footer could change and all three would stay green while every consumer got types for a package that no longer existed. tests/bundles.mjs reads the runtime probe and the declaration side by side and asserts that they agree, rather than either one alone: jdate.cjs require() yields the class d.cts says export = jdate.mjs only default is exported d.mts says export default index.d.ts export =, matching "main" jdate.js a fresh vm context is left holding the class jdate.min.js the same, with the name surviving minification Each bundle is exercised as well as inspected -- constructed, formatted both numerically and by name, and asked for a static -- because an empty class would satisfy every structural assertion on its own. The name check is what would notice keepNames going missing from the build, which is otherwise invisible until a stack trace reads _JDate or a single letter. The declaration side is matched with a line-anchored pattern. The shared body in types/jdate.d.ts names both footer shapes in prose, so an unanchored search finds whichever it looks for in every file and could never fail. Not a vitest suite. `npm test` covers src/ and has to work on a fresh clone, where lib/ does not exist yet; vitest collects only files ending in .test.js, so the .mjs name keeps this one out of that run. It gets its own script beside the two that already read lib/, and CI runs the three together after the build. Verified by breaking each thing it guards, one at a time, and confirming the failure lands on the check that should catch it and on no other: the cjs footer removed, the d.cts footer swapped for the ESM one, the iife footer removed, and a named export added to the mjs bundle. All four are caught, exit status is 1, and the healthy tree passes with 0. No runtime code changed, and lib/ is byte for byte what it was.
Review of the previous commit found the index.d.ts check asserting a
constant while claiming to assert a correspondence. It read that file and
compared its footer against a hardcoded "export =", never looking at
package.json, so it proved only that a file of that name was CJS-shaped --
not that "types" pointed at it, nor that "main" was CJS.
Aiming "types" at ./lib/jdate.d.mts while "main" stayed ./lib/jdate.cjs
passed it 4/4, and passed attw too: the node10 check confirms that types
resolve, not that their shape matches the implementation. A consumer on
moduleResolution: node would have been handed `export default` types for a
bundle that hands back the class, which is the one mistake this file exists
to catch.
So the pairs are now read out of package.json -- "main" with "types", and
each condition of exports["."] with the types beside it -- and each one is
checked by probing what loading the implementation actually yields and
comparing that against the declaration's footer. Neither side is hardcoded,
and the exports map is covered rather than assumed. A missing condition
throws rather than being skipped, since a pair that quietly left the map
would otherwise take its check with it.
Which shape the CJS bundle has is no longer asserted by the pairing, because
either shape is one a declaration may legitimately state; what must not
happen is the two disagreeing. That the bundle hands back the class and not
{ default: JDate } is a deliberate interop guarantee -- the README documents
`const JDate = require('jalali-date')` -- so it is pinned in a check of its
own rather than riding on the pairing.
Two smaller things from the same review. Running this without a build gave
four ENOENT failures and no hint; it now says which build to run, once,
before any check. And unexpected errors were reported as their first message
line with the stack dropped, which is the wrong way round: an assertion
carries its own message, while a bundle that throws on load has the stack as
its whole story. Assertions still print one line; everything else prints the
stack.
The header comment credited vitest with collecting only *.test.js. That is
this project's include pattern in vitest.config.mjs, not vitest's own
behaviour -- under its defaults a file named bundles.test.mjs would be
collected -- so the comment now points at the config.
Verified by breaking each guarded thing again, one at a time: "types"
crossed with "main", exports import.types crossed with the ESM bundle, the
require condition deleted from the map, the cjs footer removed, the d.cts
footer swapped, the iife footer removed, and a named export added to the mjs
bundle. All seven are caught, each by the check that should catch it, with
the mismatch named in the message. lib/ absent gives one line and exit 1; a
throwing bundle gives a full stack; the healthy tree passes 3/3 with exit 0.
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
1 participant
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.
Closes the gap called out under "Not included" in #71.
That change shipped declarations describing an export shape for each bundle, and left nothing checking the bundles still had it. Each existing check looks at one side only:
tests/types/compiles against the declarations, so it sees only what they claim.attwchecks that the entry points resolve to types — not what those types export. Swapping the CJS footer for the ESM one leaves it fully green.src/and never loadslib/at all.So a build footer could change and all three would stay green while every consumer got types for a package that no longer existed.
What it does
tests/bundles.mjsreads the runtime probe and the declaration side by side, and asserts that they agree — rather than either one alone.jdate.cjsrequire()yields the class, no.default.d.ctssaysexport =jdate.mjs['default'].d.mtssaysexport defaultjdate.js,jdate.min.jsvmcontext is left holding the class"main"points at the CJS bundleindex.d.tssaysexport =Each bundle is exercised as well as inspected — constructed, formatted both numerically and by name, and asked for a static — because an empty class would satisfy every structural assertion on its own. The
name === 'JDate'check is also what would noticekeepNamesgoing missing from the build config, which is otherwise invisible until a stack trace reads_JDateor a single letter.The declaration side is matched with a line-anchored pattern. The shared body in
types/jdate.d.tsnames both footer shapes in prose, so an unanchored search would find whichever it looked for in every file and could never fail.Why not a vitest suite
npm testcoverssrc/and has to work on a fresh clone, wherelib/does not exist yet. Vitest collects only files ending in.test.js, so the.mjsname keeps this one out of that run — still 3 files / 101 tests. It gets its own script beside the two that already readlib/, and CI runs the three together after the build.Verification
Broke each thing it guards, one at a time, and confirmed the failure lands on the check that should catch it and on no other:
expected the class, got object.d.ctsfooter swapped to ESMdeclares a shape the cjs bundle does not haveexpected the class, got objectunexpected named exportsAll four caught, exit status 1; the healthy tree exits 0. Two failure messages were uninformative on the first pass (a bare
Expected values to be strictly equal), so those asserts got explicit messages.No runtime code changed, and
lib/is byte for byte what it was.