Skip to content

Conversation

@strawburster
Copy link

@strawburster strawburster commented Feb 7, 2026

Description

Cache eslint runs so that they will run faster in the future using the local file ./.eslintcache

Instructions for Reviewers

Build the angular development image and run the linter on it:

tag=dspace/dspace-angular:eslint-cache
podman build . --tag $tag
time podman run --entrypoint '' $tag npm run lint

On my machine this is 0m3.580s

vs latest docker angular development build:

tag=dspace/dspace-angular:latest
podman pull $tag
time podman run --entrypoint '' $tag npm run lint

On my machine it is 1m25.783

The image size is increased by 20.1 megabytes as a result of the .eslintcache file being included, and the initial build time for the angular image will be increased by approximately 90 seconds.

List of changes in this PR:

  • Dockerfile: run linter as part of build process for development image
  • package.json: add --cache=true to lint scripts to build and use the .eslintcache file

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You do not need to complete this checklist prior creating your PR (draft PRs are always welcome).
However, reviewers may request that you complete any actions in this list if you have not done so. If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

  • My PR is created against the main branch of code (unless it is a backport or is fixing an issue specific to an older branch).
  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes ESLint validation using npm run lint
  • My PR doesn't introduce circular dependencies (verified via npm run check-circ-deps)
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • My PR aligns with Accessibility guidelines if it makes changes to the user interface.
  • My PR uses i18n (internationalization) keys instead of hardcoded English text, to allow for translations.
  • My PR includes details on how to test it. I've provided clear instructions to reviewers on how to successfully test this fix or feature.
  • If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
  • If my PR fixes an issue ticket, I've linked them together.

Add eslint cache to docker image, even if there are linting errors
@alanorth
Copy link
Contributor

alanorth commented Feb 9, 2026

Thanks @strawburster. Results for me on Linux with Node.js v24 on main:

Before:

  • npm run lint 92.26s user 3.11s system 154% cpu 1:01.88 total

After (first run to build cache, second run to test):

  • npm run lint 92.25s user 3.19s system 154% cpu 1:01.79 total
  • npm run lint 3.15s user 0.40s system 181% cpu 1.958 total

I rarely run lint in my day to day DSpace maintenance activities. The huge benefit here seems to be speeding up our CI runs. Are there any downsides?

P.S. props for using podman instead of Docker. I do too!

@tdonohue tdonohue added code task 1 APPROVAL pull request only requires a single approval to merge labels Feb 10, 2026
@tdonohue tdonohue moved this to 🙋 Needs Reviewers Assigned in DSpace 10.0 Release Feb 10, 2026
@strawburster
Copy link
Author

Hey, thanks for testing! Actually, I wasn't concerning myself with the CI pipeline, but just for local development. Do we use the main Dockerfile for CI? It says it's for development purposes. If we do, then, I don't think this change will actually improve speed because creating the lint cache takes just as much time, if not more, than linting already, so the time gained in the actual test portion is more than lost in the build portion.

I usually run the dev server and linter inside the container, and bind-mount just the source code, that way I can avoid installing node_modules dependencies on my host system. The point of adding the lint-cache command to the dockerfile was so that I could easily delete and recreate containers, with the cache file stored in the image, and the first lint task I run will be very fast.

One of the issues I ran into is that the metadata between the image files and the bind-mount files is usually not correct, so the cache-strategy must be set to 'content' to ensure that it is used during a bind-mount. And, instead of dirtying the worktree more with the cache file, I moved it to the .angular folder. This makes it easy to exclude the angular build cache and the eslint cache at once from a bind mount. In the default case, where the eslintcache is stored as a file in the root of the project, it's impossible to exclude a single file from a bind-mount while you can ignore whole folders.

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

Labels

1 APPROVAL pull request only requires a single approval to merge code task

Projects

Status: 🙋 Needs Reviewers Assigned

Development

Successfully merging this pull request may close these issues.

3 participants