fix: clean dist before build and retry throttled reader responses - #125
Merged
Conversation
Three release-blocking issues found while reviewing v7 end to end. `yarn build` wrote into whatever `dist/` was already there, so files deleted from `src` survived in the published package: 7.0.0 packed with `dist/transactions/helper.js` (deleted in #123) and `dist/version.js` (deleted in #124), both reachable through the `./dist/*` exports pattern. `prepublishOnly` runs `build`, so the fix is a clean step in `build` itself. A reader node under load answers 429 or 503. `handleFetch` only resolves a body on HTTP 200, so those answers arrived at callers as `undefined` and surfaced as `Error: Contract not found` -- observed against the app's `/api/domains/recent` and `/api/domains/stats` routes while running its end-to-end suite. Retry them. 404 still falls through to `undefined`, which is how a missing AVL value is reported. `privateKeyToAddress` and the signing backends had no exports entry, leaving `@metanames/sdk/dist/transactions` as the only way in. Add `./transactions`.
|
Total Coverage: 91.87% Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
6.3.1 shipped no exports map, so `@metanames/sdk/dist/models` and its siblings resolved through plain file lookup. The exports map added in v7 turns those into `./dist/models.js`, which does not exist, and Node does not fall back to a directory index. Consumers on those specifiers would break on upgrade for no reason: each directory has an `index.js`, so the entries point at it. Claude-Session: https://claude.ai/code/session_01GceWCwGXu66D1xEBDxZWBb
|
Total Coverage: 91.86% Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Working notes committed by accident in the previous commit. #125 squash merges, so main never carries them. Claude-Session: https://claude.ai/code/session_01GceWCwGXu66D1xEBDxZWBb
|
Total Coverage: 91.86% Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
`./dist/*` was carried over so consumers importing the build output kept working. That advertises the build layout as API: the paths hard-code the CJS tree, and nothing outside it can move without breaking someone. v7 is the major to stop. The named subpaths cover every use. Claude-Session: https://claude.ai/code/session_01GceWCwGXu66D1xEBDxZWBb
|
Total Coverage: 91.86% Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
Found while reviewing the whole SDK before cutting v7, and while running the React app's suites against a v7 tarball.
1. Stale files ship in the published package (release blocker)
buildisbuild:cjs && build:esm, both plaintscruns into an existingdist/. Nothing removes output for sources that no longer exist. A tarball packed from this tree today contains:prepublishOnlyrunsbuild, so a realnpm publishof 7.0.0 would have shipped dead modules that referenceShardedClientmethods #120 removed — and the./dist/*exports pattern makes them importable. Fixed with acleanstep insidebuild.2. A throttled reader node reports as a missing contract
handleFetchresolves a body only on HTTP 200 and returnsundefinedfor everything else, with no retry above the fetch itself. A 429 or 503 therefore reachesgetContractFromRegistryas "no contract", which throwsContract not found.Observed for real: running the app's Playwright suite (4 workers, live testnet) produced repeated
Error: Contract not foundfromsdk.domainRepository.getAll()in/api/domains/recentand/api/domains/stats, while the same call from a single process succeeds (1313 domains). Twodns-recordsspecs failed behind it.429 and 5xx are now retried by the existing
promiseRetrychain. 404 and other 4xx still fall through toundefined— that path is howgetContractStateAvlValuereports a missing key, and it is unchanged.3.
./transactionsis not in the exports mapv7 adds
privateKeyToAddress(the replacement for the app'spartisia-blockchain-applications-cryptousage) and exports the four signing backends fromsrc/transactions/index.ts, but the only way to reach them was the deep@metanames/sdk/dist/transactions. Added a./transactionssubpath.Verification
yarn buildthenls dist/transactions— nohelper.jsyarn test— 269/269 green against live testnetnpx tsc --noEmit,npx eslint src— cleandev/sdk-v7branch installed from a v7 tarball:tsc --noEmitclean, 766/766 vitest,next buildgreen, 157/162 e2e (remaining failures are app-side color-contrast a11y, unrelated)