-
Notifications
You must be signed in to change notification settings - Fork 2
fix: pdf download link not displayed correctly #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 5c82e54 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis update introduces workflow enhancements, documentation improvements, type coverage tooling, and minor code adjustments. Workflow YAML files are updated for parallelization, PDF handling, and compact publishing. The README gains badges and license details, while the MIT license is formally added. Code changes focus on type coverage directives and type safety improvements, with no public API modifications. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GitHub Actions
participant Docs Builder
participant PDF Exporter
participant Deploy Step
User->>GitHub Actions: Push/PR triggers workflow
GitHub Actions->>Docs Builder: yarn docs:build -e -p doom
Docs Builder-->>GitHub Actions: Build docs as PDFs
GitHub Actions->>PDF Exporter: yarn docs:export -p doom
PDF Exporter-->>GitHub Actions: Export PDFs to temp
GitHub Actions->>Docs Builder: yarn docs:build -d -p doom
GitHub Actions->>PDF Exporter: Copy PDFs to dist
GitHub Actions->>Deploy Step: Deploy with CNAME doom.js.org
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the PDF download link not displaying correctly while adding several type improvements and workflow updates, as well as updating documentation and license information.
- Refactors ref-collection loops with explicit type casts and null checks.
- Updates error handling in release note fetching and refines nav menu logic in the version selector.
- Enhances CI/CD workflows and updates package metadata and documentation.
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/runtime/components/OpenAPIRef.tsx | Added type casts and null checks in the recursive ref collection logic. |
| src/runtime/components/OpenAPIPath.tsx | Similar type refinements as in OpenAPIRef for safe object iteration. |
| src/plugins/replace/resolve-release.ts | Cast error to typed variable for clarity, though a potential undefined variable is still referenced. |
| src/plugins/replace/resolve-reference.ts | Introduced explicit type annotation for variable "publicBase". |
| src/plugins/replace/index.ts | Added an inline ignore comment for type-coverage on logging errors. |
| src/global/VersionsNav/index.tsx | Revised the nav menu update logic to better handle theme changes and PDF download link display. |
| src/cli/merge-pdfs/index.ts | Added an inline ignore comment to control type-coverage checks for NODEFS usage. |
| package.json | Added MIT license and introduced type-coverage configuration with additional scripts. |
| README.md | Updated documentation links and added new badges for build status, type coverage, and more. |
| LICENSE | Added the MIT License file. |
| .github/workflows/pkg-pr-new.yml | Updated Publish job command with the "--compact" flag. |
| .github/workflows/gh-pages.yml | Revised docs build/export process to handle PDFs and set a custom domain. |
| .github/workflows/ci.yml | Consolidated build, lint, and type coverage checks into a single run command. |
Comments suppressed due to low confidence (1)
src/plugins/replace/resolve-release.ts:124
- The variable 'data' is not defined in this scope. Consider replacing 'data' with 'error.data', making sure the intended error payload is correctly logged.
`${error.data ? `data ${JSON.stringify(data, null, 2)}` : `message `${error.message}`}`
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/plugins/replace/resolve-reference.ts (1)
39-53: Suggest optional refactor: collapsepublicBaseinitialization
You’ve added alet publicBase: stringfor clarity. You could simplify by using a singleconst publicBase = source.repo ? ... : ...expression to reduce mutable bindings:- let publicBase: string + const publicBase = source.repo ? path.resolve(repoFolder, source.publicBase ?? 'docs/public') : localPublicBaseThis eliminates the separate declaration and improves readability.
package.json (1)
127-141: Type coverage configuration
ThetypeCoverageblock enforces 100% coverage with strict rules. Consider removing"update": truefrom the CI configuration to avoid unintended baseline updates—run updates manually when thresholds legitimately change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
.changeset/weak-actors-attend.md(1 hunks).github/workflows/ci.yml(1 hunks).github/workflows/gh-pages.yml(1 hunks).github/workflows/pkg-pr-new.yml(1 hunks)LICENSE(1 hunks)README.md(1 hunks)package.json(4 hunks)src/cli/merge-pdfs/index.ts(1 hunks)src/global/VersionsNav/index.tsx(1 hunks)src/plugins/replace/index.ts(1 hunks)src/plugins/replace/resolve-reference.ts(1 hunks)src/plugins/replace/resolve-release.ts(1 hunks)src/runtime/components/OpenAPIPath.tsx(1 hunks)src/runtime/components/OpenAPIRef.tsx(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~9-~9: Using many exclamation marks might seem excessive (in this case: 6 exclamation marks for a text that’s 1060 characters long)
Context: ....svg)](https://conventionalcommits.org)
[
🔇 Additional comments (19)
src/runtime/components/OpenAPIRef.tsx (1)
138-140: Good defensive programming improvement!The additional nullish check and type assertion enhance robustness when processing OpenAPI schemas. This prevents potential runtime errors when encountering
nullorundefinedvalues in the schema object.src/runtime/components/OpenAPIPath.tsx (1)
149-151: Great consistency in applying the fix!This change correctly adds the same defensive programming pattern as in the OpenAPIRef component. The type assertion to
unknown[]and nullish check ensure safe traversal of the schema object, preventing errors when processing undefined or null values.src/plugins/replace/index.ts (1)
101-101: Appropriate use of type coverage ignore directive.The added directive helps maintain type coverage reporting without altering runtime behavior. This is a reasonable approach for this error handling case where the error type might be unpredictable.
src/plugins/replace/resolve-release.ts (1)
122-124: Good type safety improvement.Creating a properly typed error variable improves type safety when accessing properties like
response.status,data, andmessage. This change makes the code more robust from a static analysis perspective without changing runtime behavior..changeset/weak-actors-attend.md (1)
1-5: Appropriate changeset for the fix.The changeset correctly documents this as a patch-level update addressing the PDF download link display issue, which aligns with the changes in the VersionsNav component.
src/cli/merge-pdfs/index.ts (1)
33-39: Approve type-coverage ignore directive
The// type-coverage:ignore-linecomment on thepyodide.FS.filesystems.NODEFSmount is consistent with other ignores in the repo and avoids noise from this untyped API..github/workflows/pkg-pr-new.yml (1)
35-35: Activate compact publishing flag
Enabling--compactonyarn dlx pkg-pr-new publishstreamlines the output as intended. Ensure downstream consumers aren’t relying on the previous verbose output format..github/workflows/ci.yml (1)
39-39: Consolidate build, docs, lint, and type coverage
Combining tasks in one step (yarn run-p build docs lint typecov) reduces step overhead. This change aligns with new npm scripts and keeps the CI concise.LICENSE (1)
1-22: Add MIT License
Including the full MIT License file and matching thelicense: MITfield inpackage.jsoncorrectly formalizes the project’s licensing terms.README.md (3)
3-13: Badges addition is clear and informative
The new CI, type coverage, CodeRabbit, npm, and other badges improve project visibility and status tracking.🧰 Tools
🪛 LanguageTool
[style] ~9-~9: Using many exclamation marks might seem excessive (in this case: 6 exclamation marks for a text that’s 1060 characters long)
Context: ....svg)](https://conventionalcommits.org)![]()
[
19-29: Changelog and License sections added
Introducing the “Changelog” and “License” sections with links to CHANGELOG.md and the MIT license ensures better transparency and compliance.
17-17:Details
✅ Verification successful
Docs URL updated
The link to https://doom.js.org/ replaces the old domain. Please verify that the custom domain is correctly configured and accessible.You can run:
to confirm an HTTP 200 response.
🏁 Script executed:
curl -Is https://doom.js.org/ | head -n 1Length of output: 55
Docs URL verified and accessible
Rancurl -Is https://doom.js.org/ | head -n1→HTTP/2 200, confirming the custom domain is correctly configured and the link is live..github/workflows/gh-pages.yml (2)
31-44: Multi-step documentation build & PDF export
The workflow now exports the PDFs, rebuilds the docs with download links, and reinserts them intodist. The ordering and temporary storage steps are correct and robust.
51-51: Custom domain (cname) configured
Specifyingcname: doom.js.orgensures the site is served under the correct custom domain.package.json (5)
29-36: Keywords updated
The new keywords related to documentation and plugins enhance discoverability on npm.
41-44: Documentation scripts introduced
Thedocs,docs:build, anddocs:exportscripts align with CI workflows and replace the old export command.
55-55: Type coverage script added
Introducing thetypecovscript (type-coverage) integrates type coverage checks into your CI pipeline.
119-119:type-coveragedevDependency added
Lockingtype-coverageindevDependenciesensures reproducible type coverage runs.
9-9:Details
✅ Verification successful
License field added
Adding"license": "MIT"is essential for clarity. Please ensure a LICENSE file exists at the project root.You can verify with:
🏁 Script executed:
test -f LICENSE || echo "LICENSE file missing"Length of output: 17
License file verified
TheLICENSEfile is present at the project root, so adding"license": "MIT"inpackage.jsonis correct. No further action needed.
Summary by CodeRabbit
Documentation
Chores
Bug Fixes
Style