Skip to content

chore(repo): migrate from yarn to pnpm#1598

Open
alexhb1 wants to merge 1 commit into
grimmory-tools:developfrom
alexhb1:chore/pnpm
Open

chore(repo): migrate from yarn to pnpm#1598
alexhb1 wants to merge 1 commit into
grimmory-tools:developfrom
alexhb1:chore/pnpm

Conversation

@alexhb1
Copy link
Copy Markdown
Member

@alexhb1 alexhb1 commented Jun 1, 2026

Description

Migrates frontend tooling from yarn to pnpm, and cleans up related tools such as removing corepack and mise which are no longer needed.

Linked Issue

Fixes #1471

Changes

  • Yarn 4 > pnpm 11.3.0, with yarn.lock and yarnrc.yml replaced with pnpm-lock.yaml and pnpm-workspace.yaml.
  • New package.json in root covering frontend and release tools
  • Removed corepack. It'll be removed in Node 26 anyway and pnpm can be managed directly.
  • Removed unused mise.toml, and cleaned up duplicate hardcoded node versions across the repo, leaving .nvrmc as the single source for node versions
  • Updated justfile, dockerfile, dev compose and CI jobs to use pnpm.
  • Updated dependabot to work with single-lockfile pnpm install
  • Cleaned up odd dependency overrides that were broken in yarn and fixed transitive dependency in virtual-grid.util.ts.

Manual Testing Steps

Repo UI install, confirm all checks and tests work, dev server launches, dependencies are correctly installed for both dev environments and in the docker build.

Screenshots (Optional)

N/A

Additional Context (Optional)

N/A

AI Disclosure

N/A

Checklist

  • This PR links and implements an accepted issue.
  • This PR is a single focused change.
  • There are new or updated tests validating this change.
  • I ran just ui check and just api check.
  • I have added screenshots if there were any UI changes.
  • I have disclosed any AI usage as per the organization AI Policy above.
  • I understand all of my submitted changes.

Summary by CodeRabbit

  • Chores
    • Migrated project package manager from Yarn to pnpm across build systems, CI/CD workflows, and development environments.
    • Updated development setup documentation to reflect new package manager requirements.
    • Updated Docker and containerized build configurations to use pnpm.
    • Consolidated workspace configuration and removed legacy package manager settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 1, 2026

Review Change Stack

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 20076ebe-7293-4a56-8569-f379da520d7e

📥 Commits

Reviewing files that changed from the base of the PR and between 02cdaef and 3345162.

⛔ Files ignored due to path filters (3)
  • frontend/yarn.lock is excluded by !**/yarn.lock, !**/*.lock, !**/yarn.lock
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml, !**/pnpm-lock.yaml
  • tools/release/yarn.lock is excluded by !**/yarn.lock, !**/*.lock, !**/yarn.lock
📒 Files selected for processing (25)
  • .coderabbit.yaml
  • .dockerignore
  • .github/actions/compute-release-notes/action.yml
  • .github/dependabot.yml
  • .github/workflows/angular-lint-threshold.yml
  • .github/workflows/release-main.yml
  • .github/workflows/release-preview.yml
  • .github/workflows/test-suite.yml
  • .gitignore
  • .yarnrc.yml
  • DEVELOPMENT.md
  • Dockerfile
  • Justfile
  • dev.docker-compose.yml
  • frontend/.gitignore
  • frontend/.yarnrc.yml
  • frontend/Justfile
  • frontend/package.json
  • frontend/playwright/README.md
  • frontend/src/app/shared/util/virtual-grid.util.ts
  • mise.toml
  • package.json
  • pnpm-workspace.yaml
  • tools/release/Justfile
  • tools/release/package.json
 _____________________________________________
< My `#FF570A` highlighter marks all the TODOs. >
 ---------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

Warning

Ignoring CodeRabbit configuration file changes. For security, only the configuration from the base branch is applied for open source repositories.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 1, 2026

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
{}

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/package.json (1)

1-88: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Verify removed dependency overrides are resolved safely in pnpm-lock.yaml.

The PR removed the overrides block that previously forced specific versions of security-sensitive dependencies:

  • minimatch^10.2.1
  • ajv^8.18.0
  • eslint > ajv^6.14.0
  • undici^7.24.1

pnpm's resolution algorithm differs from Yarn's. Confirm these transitive dependencies resolve to patched versions in the new lockfile.

#!/bin/bash
# Check resolved versions of previously-overridden dependencies in pnpm-lock.yaml
echo "Checking critical dependency versions in pnpm-lock.yaml:"
echo ""

if [ ! -f pnpm-lock.yaml ]; then
  echo "ERROR: pnpm-lock.yaml not found"
  exit 1
fi

echo "=== minimatch versions ==="
rg -A2 "^  minimatch@" pnpm-lock.yaml | rg "version:" | head -5

echo ""
echo "=== ajv versions ==="
rg -A2 "^  ajv@" pnpm-lock.yaml | rg "version:" | head -5

echo ""
echo "=== undici versions ==="
rg -A2 "^  undici@" pnpm-lock.yaml | rg "version:" | head -5

echo ""
echo "Checking for known vulnerable versions:"
echo "- minimatch <10.2.0 (ReDoS vulnerability)"
echo "- ajv <8.17.1 (prototype pollution)"
echo "- undici <7.24.0 (SSRF vulnerability)"
Are there known security vulnerabilities in minimatch versions below 10.2.1, ajv versions below 8.18.0, or undici versions below 7.24.1?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/package.json` around lines 1 - 88, Removed overrides for minimatch,
ajv, undici (and eslint->ajv) may allow vulnerable transitive versions; inspect
pnpm-lock.yaml to confirm resolved versions and reintroduce pins if needed. Open
pnpm-lock.yaml and search for the package keys "minimatch@", "ajv@" and
"undici@" (and any "eslint" entries that reference ajv) to note the resolved
"version:" entries; if any resolved versions are older than the previously
enforced safe versions (minimatch >=10.2.1, ajv >=8.18.0, undici >=7.24.1)
regenerate the lockfile after updating package constraints or reintroduce an
"overrides" block in package.json to pin those packages to the safe versions,
then run pnpm install to update pnpm-lock.yaml and re-run the same verification
to ensure the lockfile now contains the patched versions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@DEVELOPMENT.md`:
- Line 87: Update the prerequisites table row that currently reads "Node.js +
pnpm | 24+" to explicitly pin the required pnpm version (for example "Node.js
24+ + pnpm 11.3.0+") so readers know the exact pnpm minimum; locate the table
entry containing the text "Node.js + pnpm" in the README/DEVELOPMENT content and
replace the generic "24+" pnpm note with a precise pnpm version string and an
optional "+", e.g., "pnpm 11.3.0+".

---

Outside diff comments:
In `@frontend/package.json`:
- Around line 1-88: Removed overrides for minimatch, ajv, undici (and
eslint->ajv) may allow vulnerable transitive versions; inspect pnpm-lock.yaml to
confirm resolved versions and reintroduce pins if needed. Open pnpm-lock.yaml
and search for the package keys "minimatch@", "ajv@" and "undici@" (and any
"eslint" entries that reference ajv) to note the resolved "version:" entries; if
any resolved versions are older than the previously enforced safe versions
(minimatch >=10.2.1, ajv >=8.18.0, undici >=7.24.1) regenerate the lockfile
after updating package constraints or reintroduce an "overrides" block in
package.json to pin those packages to the safe versions, then run pnpm install
to update pnpm-lock.yaml and re-run the same verification to ensure the lockfile
now contains the patched versions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 20076ebe-7293-4a56-8569-f379da520d7e

📥 Commits

Reviewing files that changed from the base of the PR and between 02cdaef and 3345162.

⛔ Files ignored due to path filters (3)
  • frontend/yarn.lock is excluded by !**/yarn.lock, !**/*.lock, !**/yarn.lock
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml, !**/pnpm-lock.yaml
  • tools/release/yarn.lock is excluded by !**/yarn.lock, !**/*.lock, !**/yarn.lock
📒 Files selected for processing (25)
  • .coderabbit.yaml
  • .dockerignore
  • .github/actions/compute-release-notes/action.yml
  • .github/dependabot.yml
  • .github/workflows/angular-lint-threshold.yml
  • .github/workflows/release-main.yml
  • .github/workflows/release-preview.yml
  • .github/workflows/test-suite.yml
  • .gitignore
  • .yarnrc.yml
  • DEVELOPMENT.md
  • Dockerfile
  • Justfile
  • dev.docker-compose.yml
  • frontend/.gitignore
  • frontend/.yarnrc.yml
  • frontend/Justfile
  • frontend/package.json
  • frontend/playwright/README.md
  • frontend/src/app/shared/util/virtual-grid.util.ts
  • mise.toml
  • package.json
  • pnpm-workspace.yaml
  • tools/release/Justfile
  • tools/release/package.json
💤 Files with no reviewable changes (5)
  • .yarnrc.yml
  • mise.toml
  • .coderabbit.yaml
  • tools/release/package.json
  • frontend/.yarnrc.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Test Suite / Backend Tests
  • GitHub Check: Test Suite / Frontend Tests
  • GitHub Check: Analyze (java-kotlin)
  • GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (6)
**/*

⚙️ CodeRabbit configuration file

**/*: This project is being developed using current and future-facing technologies:

  • Java 25 with --enable-preview (preview features are INTENTIONAL and encouraged)
  • Spring Boot 4 (latest major version, check APIs accordingly)
  • Jackson 3 (new package: tools.jackson.* instead of com.fasterxml.jackson.*)
  • Hibernate 7.3.x (Jakarta Persistence 3.2, new APIs; avoid deprecated Hibernate 5/6 patterns)
  • Angular 21 (signals-based reactivity, no NgModules unless legacy)

Grimmory Internal Tools

Metadata Standards and Compliance

  • For all metadata writing and parsing logic, double-check against Dublin Core and ANSI standards to ensure perfect official compliance.
  • We strictly follow the widespread and official XML-compliant methods for EPUB2, EPUB3, CBX, and PDF formats.

General Java and Spring rules

  • ALWAYS prefer modern, idiomatic Java 25 constructs over legacy patterns.
  • Preview features (--enable-preview) are enabled and intentional; do NOT flag them as risky unless there is a concrete runtime issue.
  • Prefer: records, sealed classes/interfaces, pattern matching (switch expressions, instanceof), structured concurrency (StructuredTaskScope), scoped values, string templates, unnamed patterns/variables.
  • Prefer virtual threads (Thread.ofVirtual(), Executors.newVirtualThreadPerTaskExecutor()) over platform threads for I/O-bound work.
  • Prefer the new Sequenced Collections API (SequencedCollection, SequencedMap) where applicable.
  • Prefer var for local variables when the type is obvious from context.
  • Use stream().toList() instead of stream().collect(Collectors.toList()) for imm...

Files:

  • package.json
  • frontend/playwright/README.md
  • DEVELOPMENT.md
  • Justfile
  • Dockerfile
  • pnpm-workspace.yaml
  • dev.docker-compose.yml
  • frontend/src/app/shared/util/virtual-grid.util.ts
  • tools/release/Justfile
  • frontend/package.json
  • frontend/Justfile
**/.github/workflows/*.yml

⚙️ CodeRabbit configuration file

**/.github/workflows/*.yml: GitHub Actions review:

  • Pin action versions to SHA hashes, not floating tags.
  • Flag secrets referenced as env vars in run: steps; prefer secrets context.
  • Flag missing permissions: blocks (principle of least privilege).
  • Ensure Java 25 is configured in the workflow, and verify --enable-preview is passed where the JVM is actually launched (for example via Gradle/Maven args, JAVA_TOOL_OPTIONS, or the explicit java command).

Files:

  • .github/workflows/test-suite.yml
  • .github/workflows/angular-lint-threshold.yml
  • .github/workflows/release-main.yml
  • .github/workflows/release-preview.yml
**/Dockerfile*

⚙️ CodeRabbit configuration file

**/Dockerfile*: Dockerfile review:

  • Prefer multi-stage builds (builder + runtime).
  • Use non-root USER in the final stage.
  • Pin base image versions (e.g., eclipse-temurin:25-jre-noble).
  • Flag secrets or credentials embedded in the image.
  • Ensure --enable-preview JVM flag is carried into ENTRYPOINT/CMD if required at runtime.

Files:

  • Dockerfile
frontend/src/**/*.{ts,tsx,html,scss}

📄 CodeRabbit inference engine (AGENTS.md)

Use 2-space indentation in TypeScript, HTML, and SCSS in frontend code

Files:

  • frontend/src/app/shared/util/virtual-grid.util.ts
frontend/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Prefer inject() over constructor injection in frontend code

Files:

  • frontend/src/app/shared/util/virtual-grid.util.ts
frontend/src/**/*.{ts,tsx,html}

📄 CodeRabbit inference engine (AGENTS.md)

frontend/src/**/*.{ts,tsx,html}: Follow frontend/eslint.config.js: component selectors use app-, directive selectors use app, and any is disallowed in frontend code
Put user-facing strings in Transloco files under frontend/src/i18n/

Files:

  • frontend/src/app/shared/util/virtual-grid.util.ts
🧠 Learnings (16)
📚 Learning: 2026-05-18T14:54:39.422Z
Learnt from: alexhb1
Repo: grimmory-tools/grimmory PR: 1379
File: frontend/src/assets/styles/tailwind.css:3-4
Timestamp: 2026-05-18T14:54:39.422Z
Learning: In the grimmory-tools/grimmory repository, Biome is not used for linting/formatting (no `biome.json` and no Biome dependency in `package.json`). During code reviews, do not raise Biome-related issues or recommend adding/changing `biome.json`/Biome dependencies for formatting or linting in this project.

Applied to files:

  • package.json
  • frontend/playwright/README.md
  • DEVELOPMENT.md
  • frontend/src/app/shared/util/virtual-grid.util.ts
  • frontend/package.json
📚 Learning: 2026-03-25T21:02:55.427Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 195
File: booklore-api/src/main/java/org/booklore/config/logging/RequestLoggingFilterConfig.java:15-19
Timestamp: 2026-03-25T21:02:55.427Z
Learning: In `booklore-api/src/main/java/org/booklore/config/logging/RequestLoggingFilterConfig.java` and `booklore-api/src/main/java/org/booklore/config/logging/filter/RequestLoggingFilter.java`, do not require header redaction or payload scrubbing during review. Logging all request headers and payloads at `DEBUG` level is intentional for this project. The filter is not profile-gated (unlike the prior `Profile("dev")` behavior) and should only emit logs when `logger.isDebugEnabled()` is true—treat that as the access control/guarding mechanism rather than expecting additional gating.

Applied to files:

  • .dockerignore
  • .gitignore
📚 Learning: 2026-03-26T05:52:04.269Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 217
File: booklore-api/src/main/java/org/booklore/service/kobo/KoboServerProxy.java:107-108
Timestamp: 2026-03-26T05:52:04.269Z
Learning: In booklore-api/src/main/java/org/booklore/service/kobo/KoboServerProxy.java, the proxy code parses every proxy response body as JSON in executeProxyRequest() (via objectMapper.readTree(response.body())). Keep the outbound proxy requests forcing the Accept header to application/json, because non-JSON (e.g., HTML) responses would fail JSON parsing on error paths (such as sync errors). Only relax or remove this behavior if you also add explicit handling for non-JSON responses (e.g., content-type checks or a fallback parser) so parse failures can’t occur.

Applied to files:

  • .dockerignore
  • .gitignore
📚 Learning: 2026-04-23T14:49:01.089Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 790
File: backend/src/main/java/org/booklore/nativelib/NativeLibraries.java:71-77
Timestamp: 2026-04-23T14:49:01.089Z
Learning: In `backend/src/main/java/org/booklore/nativelib/NativeLibraries.java`, the `catch (Throwable)` in `NativeLibraries.runProbe()` is intentional. Treat failures during native library probing (including severe JVM-related errors) as “library unavailable” rather than propagating or flagging it as an anti-pattern. In future reviews, do not flag this `catch (Throwable)` in that method as incorrect error handling.

Applied to files:

  • .dockerignore
  • .gitignore
📚 Learning: 2026-04-24T05:28:14.493Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 783
File: backend/src/main/java/org/booklore/service/metadata/BookCoverService.java:506-517
Timestamp: 2026-04-24T05:28:14.493Z
Learning: In grimmory-tools/grimmory, preserve the existing control-flow for `AppSettings.getMaxFileUploadSizeInMb()` returning `null`: in `BookCoverService.getMaxFileUploadSizeMb()`, `FileService`, and `AuthorMetadataService`, do not throw an exception. Instead, emit a warning log and return `0L` so the subsequent size check rejects all uploads. This is intentional because the app’s error handling strips 5xx details and an admin warning is required in both cases; do not suggest changing this behavior to throwing an exception.

Applied to files:

  • .dockerignore
  • .gitignore
📚 Learning: 2026-04-25T02:36:26.141Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 848
File: frontend/src/index.html:9-9
Timestamp: 2026-04-25T02:36:26.141Z
Learning: In grimmory-tools/grimmory’s frontend/src/index.html, CSP directives should be minimal and targeted to the actual resources the app uses. Avoid adding broad directives such as `default-src 'self'` (or other defense-in-depth “blanket” CSP changes) that require wider overrides, since they can break application behavior; instead, specify only the necessary `script-src`, `style-src`, `img-src`, etc. for the resources actually used by the frontend.

Applied to files:

  • .dockerignore
  • .gitignore
📚 Learning: 2026-04-29T11:54:59.650Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 991
File: backend/src/main/java/org/booklore/model/entity/BookdropFileEntity.java:39-45
Timestamp: 2026-04-29T11:54:59.650Z
Learning: In backend/src/main/java/org/booklore/model/entity/BookdropFileEntity.java, the fields BookdropFileEntity.originalMetadata and BookdropFileEntity.fetchedMetadata are intentionally annotated with Basic(fetch = FetchType.EAGER). Since bookdrop files are short-lived and their metadata is always needed immediately in the metadata fetch/review flow, do not review-flag these LOB fields as candidates for lazy loading.

Applied to files:

  • .dockerignore
  • .gitignore
📚 Learning: 2026-05-22T03:18:51.508Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1446
File: backend/src/main/java/org/booklore/service/metadata/MetadataManagementService.java:63-64
Timestamp: 2026-05-22T03:18:51.508Z
Learning: In grimmory-tools/grimmory, treat `BookEntity` with `primaryFile == null` as an intentional valid state. In `MetadataManagementService.writeMetadataToFile`, when `primaryFile` is null and the implementation correctly skips the file write, do not flag the lack of diagnostic/logging for this null-primary-file skip as a code review issue.

Applied to files:

  • .dockerignore
  • .gitignore
📚 Learning: 2026-04-30T04:30:04.350Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1002
File: .github/workflows/publish-nightly.yml:118-124
Timestamp: 2026-04-30T04:30:04.350Z
Learning: When using `docker/metadata-action` in GitHub Actions workflows with an `images: |` multiline YAML block, it’s acceptable for some `images` entries to be empty (e.g., because a registry/login var like `vars.DOCKERHUB_REGISTRY` is unset in forks). `docker/metadata-action` should skip the empty line and still produce valid metadata without downstream failures, so you generally don’t need an additional `enable`/conditional guard solely to prevent empty-image formatting issues—verify the action is still generating usable tags/labels in the workflow output.

Applied to files:

  • .github/workflows/test-suite.yml
  • .github/workflows/angular-lint-threshold.yml
  • .github/workflows/release-main.yml
  • .github/workflows/release-preview.yml
📚 Learning: 2026-05-01T02:30:02.761Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1020
File: .github/workflows/publish-release.yml:93-101
Timestamp: 2026-05-01T02:30:02.761Z
Learning: In GitHub Actions workflows, when a secret is needed by a `run:` step, prefer passing it through the step’s `env:` (e.g., `env: MY_SECRET: ${{ secrets.MY_SECRET }}`) instead of interpolating `${{ secrets.X }}` inline inside the shell script in `run:`. When reviewing, do not apply the general guideline “flag secrets referenced as env vars in `run:` steps; prefer secrets context” if the secret is provided specifically via the `env:` block for that `run:` step. Only flag/raise concerns about `env:` secrets when they are unnecessarily broad (e.g., set at job/workflow level but only used in one step) or when the secret is passed as a command-line argument (e.g., `run: some-cmd --secret "$MY_SECRET"`).

Applied to files:

  • .github/workflows/test-suite.yml
  • .github/workflows/angular-lint-threshold.yml
  • .github/workflows/release-main.yml
  • .github/workflows/release-preview.yml
📚 Learning: 2026-05-07T07:08:25.367Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1179
File: .github/workflows/publish-release.yml:36-37
Timestamp: 2026-05-07T07:08:25.367Z
Learning: In grimmory-tools/grimmory GitHub Actions workflows, treat `secrets.DOCKERHUB_USERNAME` as intentionally stored as a Secret (to stay grouped with `secrets.DOCKERHUB_TOKEN` as one cohesive set of Docker Hub credentials). Do not recommend converting `secrets.DOCKERHUB_USERNAME` to `vars.DOCKERHUB_USERNAME`; only flag `secrets.*` for conversion when there is a clear reason that it should not be a secret (e.g., an established non-sensitive value).

Applied to files:

  • .github/workflows/test-suite.yml
  • .github/workflows/angular-lint-threshold.yml
  • .github/workflows/release-main.yml
  • .github/workflows/release-preview.yml
📚 Learning: 2026-05-22T19:22:36.898Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1403
File: .github/workflows/ci-validate.yml:38-39
Timestamp: 2026-05-22T19:22:36.898Z
Learning: For this repo’s GitHub Actions workflows, ensure every `actions/checkout` step sets `persist-credentials: false`.
During reviews, do not flag missing `persist-credentials: false` on individual `actions/checkout` steps inline. If the repo doesn’t yet have a coordinated fix in progress, ask for/track it via a single bulk tracking issue and/or a consolidated PR that updates all affected checkout steps at once.

Applied to files:

  • .github/workflows/test-suite.yml
  • .github/workflows/angular-lint-threshold.yml
  • .github/workflows/release-main.yml
  • .github/workflows/release-preview.yml
📚 Learning: 2026-05-12T07:14:14.779Z
Learnt from: alexhb1
Repo: grimmory-tools/grimmory PR: 1270
File: .github/dependabot.yml:30-31
Timestamp: 2026-05-12T07:14:14.779Z
Learning: In Dependabot configuration, the `cooldown: { default-days: N }` setting should be treated as a minimum release-age gate: Dependabot will not open dependency update PRs for versions released fewer than N days ago. It is not just a PR frequency/spacing control. When reviewing `dependabot.yml`, verify that the intended policy is a release-age delay (and that it matches the project’s expectations), not merely throttling of PR creation cadence (conceptually similar to Yarn’s `npmMinimalAgeGate`, but enforced at PR creation time rather than install time).

Applied to files:

  • .github/dependabot.yml
📚 Learning: 2026-04-05T21:16:01.715Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 385
File: frontend/src/app/app.component.ts:55-56
Timestamp: 2026-04-05T21:16:01.715Z
Learning: When reviewing code in the Grimmory frontend (Angular), prefer modern Angular patterns. Specifically: (1) Prefer `DestroyRef` with `takeUntilDestroyed(destroyRef)` for teardown in Angular v16+ instead of manually tracking `Subscription` arrays and calling `unsubscribe()` in `ngOnDestroy()`. (2) Prefer `inject()` for dependency injection over constructor injection where appropriate. (3) Prefer Angular signals (e.g., `signal`, `computed`) over `BehaviorSubject`/`Observable` for state where signals/computed values fit the use case. Flag older patterns when they can be replaced with these modern equivalents without changing behavior.

Applied to files:

  • frontend/src/app/shared/util/virtual-grid.util.ts
📚 Learning: 2026-04-07T09:28:09.587Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 393
File: frontend/src/app/features/readers/pdf-reader/pdf-reader.component.ts:255-263
Timestamp: 2026-04-07T09:28:09.587Z
Learning: In this Angular frontend (under frontend/src/app/), flag manual resource management/cleanup patterns when there is an Angular v16+ automatic alternative. Examples to prefer: (1) Instead of manually pairing document/window event listeners with stored cleanup functions (e.g., add/removeEventListener with mouseMoveCleanup/documentClickCleanup/keydownCleanup/touchCleanup fields), register teardown via DestroyRef.onDestroy(cleanupFn) (or equivalent Angular v16+ teardown mechanism). (2) Instead of storing Subscriptions in fields and explicitly unsubscribing in ngOnDestroy (e.g., annotationSaveSubscription/annotationCacheSubscription), use takeUntilDestroyed(destroyRef) (piped into the observable) or other Angular v16+ primitives. (3) If teardown is lifecycle-coupled and can be automated via DestroyRef/takeUntilDestroyed/signals (or other Angular v16+ mechanisms), prefer the automated approach over manual ngOnDestroy cleanup. Raise a review finding for the manual pattern and recommend the aut...

Applied to files:

  • frontend/src/app/shared/util/virtual-grid.util.ts
📚 Learning: 2026-04-11T03:55:57.229Z
Learnt from: zachyale
Repo: grimmory-tools/grimmory PR: 439
File: frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts:178-196
Timestamp: 2026-04-11T03:55:57.229Z
Learning: In this Angular frontend (frontend/src/app/), prefer the team’s reactive i18n “signal/computed” pattern: (1) For individual reactive translated strings, use `translateSignal()` from `jsverse/transloco`. (2) For option/label arrays that must update on language switch, create a single `activeLang` signal with `toSignal(t.langChanges$, { initialValue: t.getActiveLang() })`, then derive the arrays as `computed()` signals that read `activeLang()`. This should avoid manual `langChanges$` subscriptions and any `ngOnDestroy` subscription cleanup; prefer this over subscribing in `ngOnInit` when implementing reactive localization.

Applied to files:

  • frontend/src/app/shared/util/virtual-grid.util.ts
🔀 Multi-repo context grimmory-tools/grimmory-docs

[::grimmory-tools/grimmory-docs::] repository summary

  • package-lock.json present at repo root (large lockfile) — indicates this repo uses npm lockfile. [::grimmory-tools/grimmory-docs::package-lock.json]
  • package.json present at repo root and README local instructions use npm ci / npm start (documentation/commands still reference npm). [::grimmory-tools/grimmory-docs::README.md][::grimmory-tools/grimmory-docs::package.json]
  • .gitignore references pnpm-lock.yaml and yarn.lock and pnpm/yarn debug logs, but no pnpm-lock.yaml or yarn.lock files are present in the repository. (.gitignore lines: pnpm-lock.yaml, yarn.lock, yarn-debug.log*, yarn-error.log*, pnpm-debug.log*). [::grimmory-tools/grimmory-docs::.gitignore]

Implication: the docs site remains an npm-managed project (package-lock.json + npm instructions). The upstream repo migration to pnpm could create inconsistency for contributors or CI if docs are expected to adopt pnpm as well (lockfile/install commands differ). Recommend verifying whether docs should be migrated to pnpm or intentionally remain npm-based.

🔇 Additional comments (19)
.gitignore (1)

51-57: LGTM!

.dockerignore (1)

7-13: LGTM!

frontend/.gitignore (1)

12-12: LGTM!

frontend/playwright/README.md (1)

37-37: LGTM!

pnpm-workspace.yaml (1)

1-14: LGTM!

frontend/package.json (1)

17-17: LGTM!

Also applies to: 20-20

.github/workflows/release-main.yml (1)

52-58: LGTM!

.github/actions/compute-release-notes/action.yml (1)

57-63: LGTM!

.github/workflows/release-preview.yml (1)

57-63: LGTM!

.github/workflows/test-suite.yml (1)

128-134: LGTM!

.github/workflows/angular-lint-threshold.yml (1)

31-37: LGTM!

Also applies to: 40-40, 44-44, 50-50, 77-77

package.json (1)

6-6: Verify .nvmrc aligns with package.json engines.node: ">=24"

.nvmrc exists and contains 24, matching the node: ">=24" requirement in package.json.

Justfile (1)

92-96: LGTM!

frontend/Justfile (1)

3-97: LGTM!

tools/release/Justfile (1)

3-23: LGTM!

Dockerfile (1)

3-22: LGTM!

dev.docker-compose.yml (1)

38-53: LGTM!

Also applies to: 73-75

.github/dependabot.yml (1)

22-48: ⚡ Quick win

Update Dependabot concern: pnpm-lock parsing risk doesn’t apply here — the repo’s .github/dependabot.yml uses package-ecosystem: "npm" (not pnpm), and the committed pnpm-lock.yaml is single-document (lockfileVersion: '9.0' and ^--- count = 0).

frontend/src/app/shared/util/virtual-grid.util.ts (1)

2-2: Verify @tanstack/angular-virtual@5.0.2 exports observeElementRect, Rect, and VirtualItem.

This PR only consolidates the import source in frontend/src/app/shared/util/virtual-grid.util.ts, but the sandbox doesn’t have node_modules/@tanstack/angular-virtual installed, so the adapter’s .d.ts re-export behavior can’t be checked here. Run a typecheck/build (or inspect node_modules/@tanstack/angular-virtual/*.d.ts) to ensure those symbols are exported (typically via re-export from @tanstack/virtual-core) so the consolidated import is valid.

Comment thread DEVELOPMENT.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate frontend tooling from yarn to pnpm

1 participant