Migrate codebase from JavaScript to TypeScript - #11
Draft
corvec wants to merge 9 commits into
Draft
Conversation
- Port all modules from lib/*.mjs to src/*.ts with strict typing; shared types (MacroMap, RollLog, ResultEntry, etc.) now live in src/types.ts and are exported from the package entry point - Replace Babel + Rollup + jsdoc/tsd-jsdoc pipeline with plain tsc: dist/esm (ES modules), dist/cjs (CommonJS), dist/types (.d.ts) - Replace babel-jest with ts-jest (Jest 29); all 108 tests pass - Replace babel-eslint/fp/jsdoc ESLint setup with typescript-eslint - Resolve the Collection typedef name clash: the parser's map of RPN rolls is now MacroCollection; rollFormula's targeted collections are TargetedCollection - Remove dead window.prompt fallback in rollFormula (unreachable: UnknownVariablesError is always thrown first), fixing a crash risk in non-browser environments if it ever became reachable - Publish via package.json "files" + "exports" instead of .npmignore Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNE9fy2daRdoTNMLzmH9Rg
- stripSuffix now strips lowercase-initial collection targets (@defender), matching what the tokenizer and isVariable accept; side effects on lowercase-targeted macros now resolve correctly - Anchor isRoll to whole tokens (^\d+d\d+$) so tokens that merely contain a roll (e.g. '2d6x') fail validation instead of being silently evaluated as a roll - Add injectable RNG: evaluateFormula accepts an optional rng (() => number, defaults to Math.random), threaded through rollFormula and buildResultRange via a new options parameter; enables seeded, deterministic rolls - rollFormula now throws ErrorTypes.ValidationError on an invalid formula instead of returning an error string in result, making its failure modes consistent with UnknownVariablesError; rolls is now always a RollLog - rollFormula accepts options.maxRange (default 40) passed through to buildResultRange; documented the probe bound and the fact that buildResultRange appends to the rolls log it is given - Replace O(n^2) spread-accumulation in objectMakerReduceHelper, splitTokenList, getAllRolls, and validateFormula with linear equivalents - README: fix advanced example (foundResult.result[0]) and document the new options and thrown errors - Tests: cover rng determinism and single-consumption, maxRange, ValidationError, lowercase targets, isRoll anchoring; drop duplicated getRollMetadata test Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNE9fy2daRdoTNMLzmH9Rg
Runs on PRs and pushes to master across Node 18/20/22. Test output is appended to the job summary, and the packaged module is installed into a scratch project and imported via both ESM and CJS to validate the exports map. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNE9fy2daRdoTNMLzmH9Rg
- Preview Deploy: label a PR with 'deploy-preview' (or run manually) to build Engineer's Dice Roller against this branch's roll-a-d6 and publish it to https://demo.dicerollr.com/<short-sha>/ - a subdirectory of the gh-pages branch of corvec/roll-a-d6-basic, which backs that domain. Comments the URL on the PR. Requires the PREVIEW_DEPLOY_TOKEN secret for the cross-repo push. - Preview Cleanup: weekly (and on-demand) prune of preview directories older than a cutoff (default 30 days); never touches the production deploy at the branch root. - Export the helpers module from the package entry point so consumers (e.g., the dice roller app's getAllRolls import) no longer need deep imports into the package internals. - Add a prepare script so git-based installs of this package build dist/ automatically. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNE9fy2daRdoTNMLzmH9Rg
The repo is private, so the default GITHUB_TOKEN (scoped to this repo only) gets 'Repository not found' when fetching it. Use PREVIEW_DEPLOY_TOKEN, which therefore needs contents read access on corvec/engineers-dice-roller in addition to read/write on corvec/roll-a-d6-basic. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNE9fy2daRdoTNMLzmH9Rg
The raw checkout failure for a missing or rejected token is a cryptic 'could not read Username' git error. Probe the token against the engineers-dice-roller repo up front and report exactly what is wrong. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNE9fy2daRdoTNMLzmH9Rg
GitHub deploy keys are single-repo, so one key cannot cover both engineers-dice-roller and roll-a-d6-basic. The preview and cleanup workflows now accept either credential style: - PREVIEW_DEPLOY_TOKEN: fine-grained PAT covering both repos (takes precedence when set) - APP_DEPLOY_KEY + SITE_DEPLOY_KEY: private halves of per-repo deploy keys (read-only on engineers-dice-roller, read/write on roll-a-d6-basic), used via actions/checkout ssh-key Also install the packed roll-a-d6 tarball in the same npm install as the app's other dependencies, replacing the package.json git spec so npm never clones roll-a-d6 from the runner. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNE9fy2daRdoTNMLzmH9Rg
A multi-line SSH key in PREVIEW_DEPLOY_TOKEN makes curl itself fail (exit 43) before the readable error can be printed; tolerate the curl failure so the diagnostic message always surfaces. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNE9fy2daRdoTNMLzmH9Rg
The stale PREVIEW_DEPLOY_TOKEN secret (containing an SSH key) was taking precedence over the newly added APP_DEPLOY_KEY/SITE_DEPLOY_KEY secrets and failing. Probe the PAT first and select the auth mode dynamically: use it only when it can actually read the target repo, otherwise use the deploy keys, and fail with guidance only when neither works. Applied to both preview deploy and cleanup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNE9fy2daRdoTNMLzmH9Rg
|
🎲 Preview of Engineer's Dice Roller built with this branch: https://demo.dicerollr.com/8dccfbcf/ |
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.
Summary
This PR migrates the entire roll-a-d6 codebase from JavaScript (ESM modules with
.mjsextensions) to TypeScript, adding comprehensive type safety and improving the development experience.Key Changes
Language Migration: Converted all source files from
.mjsto.tswith full TypeScript type annotationstypes.tsmodule with shared type definitions (ResultEntry,RPNTokenList,MacroMap,RollLog,SideEffects, etc.)ResultRange,RollMetadata,ParsedAssignments, etc.)Build System Overhaul: Replaced Babel/Rollup with TypeScript compiler
tsconfig.jsonwith strict mode enabledtsconfig.esm.json) and CommonJS (tsconfig.cjs.json) buildspackage.jsonto output todist/directory with proper export configurationtypesfield pointing to generated type definitionsCode Quality Improvements:
@typescript-eslintparser and pluginsTest Files: Migrated all test files from
.mjsto.tswith proper type annotations.jsextensions (TypeScript convention)Package Configuration:
exportsfield with conditional exports for ESM/CommonJStypesfield for TypeScript consumersfilesfield to only includedist/directoryNotable Implementation Details
RandomNumberGeneratorfunction type.jsextensions (required for ESM in TypeScript)https://claude.ai/code/session_01XNE9fy2daRdoTNMLzmH9Rg