Replace grunt with npm scripts (post BS5) by crhallberg · Pull Request #4591 · vufind-org/vufind

crhallberg · 2025-09-05T19:12:57Z

Target Node v20 (older LTS).

build css
watch css
setup (install dependencies)
remove grunt

From #2624

Update relevant wiki documentation before merging
Resolve VUFIND-1514 when merging
Confirm that there are no remaining useful changes that need to be migrated in from Replace grunt with NPM scripts (see #4591) #2624

demiankatz

Thanks for getting this started, @crhallberg! See below for a couple of comments.

Also, should we go ahead and close #2624 at this point? Looks like most of what's there is obsolete at this stage, though I do notice that you added Typescript type checking. Do we still want to add that? If so, maybe we should put a TODO here to address that as a follow-up PR after we've refactored the existing Grunt functionality.

demiankatz · 2025-09-06T11:38:35Z


 // Bootstrap 5
-await cp('node_modules/bootstrap/scss/.', 'scss/vendor/bootstrap/', { recursive: true });
+await cp('node_modules/bootstrap/scss/.', 'scss/vendor/bootstrap/scss/', { recursive: true });


Do we really need the double scss in the path?

Yes, for the new scss compiler which tries to resolve the Bootstrap dependencies in this folder. ~~I can try without again with updated loadPaths.~~ Nope that didn't work.

If we can't avoid this change, I imagine we'll need to commit the result of moving those files, won't we?

demiankatz

Thanks for the progress, @crhallberg -- see below for a few new thoughts.

demiankatz · 2025-09-09T12:46:24Z


 // Bootstrap 5
-await cp('node_modules/bootstrap/scss/.', 'scss/vendor/bootstrap/', { recursive: true });
+await cp('node_modules/bootstrap/scss/.', 'scss/vendor/bootstrap/scss/', { recursive: true });


If we can't avoid this change, I imagine we'll need to commit the result of moving those files, won't we?

demiankatz · 2025-09-09T12:48:20Z

  "scripts": {
-    "build": "npm run build:css",
-    "watch": "npm run watch:css",
+    "build": "npm-run-all install:scss-dependencies build:scss",


If we're not using wildcards, could we just revert to npm run install:scss-dependencies ; npm run build:scss so we can eliminate the npm-run-all2 dependency? If you strongly feel that it's useful, I don't mind having it, but if it's not essential, it's always nice to minimize dependencies.

crhallberg · 2025-09-09T20:35:27Z

I would like feedback on 00035b1. I think this will allow new themes and mixins to automatically install and update dependencies. For example, #4031 will need to install Font Awesome this way.

It does remove the build only option. I might be able to bring it back by checking for devDependencies or adding try/catches.

cc @demiankatz @EreMaijala

demiankatz

@crhallberg, I just did another general review of all the changes, without specifically focusing on 00035b1 -- I'd have to find time to research npm workspaces to fully understand all the implications there. But for now, my review really focuses on the public interface and our current practices; I think some of the changes here could significantly impact those things. Some of those changes might be for the best, but they could also have some fairly broad-reaching implications -- I want to be sure we don't pull the rug out from under anyone.

Maybe a broader question to help get this PR done: what exactly is the overall goal of the work here, and can we constrain the scope? My preference would be to eliminate Grunt with as few changes to behavior as possible, just to move us forward an incremental step without a lot of side effects. We could then follow on from there with more strategic changes, and they might be easier to understand if planned in stages. Maybe it's not possible to do this incrementally, in which case we may have to take the consequences of making several changes at once. But my preference is to always keep things as simple and non-disruptive as possible, when possible.

demiankatz · 2025-09-10T11:50:53Z

+    console.log("- minifying...");
+
+    // @link https://github.com/twbs/bootstrap/blob/main/package.json
+    // cleancss -O1 --format breakWith=lf --with-rebase --source-map --source-map-inline-sources


This comment says --with-rebase but I don't see an equivalent option in the code below. Is that a default, or is something out of sync?

demiankatz · 2025-09-10T11:52:24Z

+  console.log("- compiling SCSS to CSS...");
+
+  // @link https://github.com/twbs/bootstrap/blob/main/package.json
+  // sass --style expanded --source-map --embed-sources --no-error-css scss/:dist/css/


This comment also seems a little bit out of sync with the code below. Is it meaningful, or an old reference?

demiankatz

Thanks for the progress here, @crhallberg. Before I do another review, do you mind browsing through my previous comments and resolving the items that have been addressed? It looks to me like there are still some outstanding ones that need addressing (e.g. regarding outdated/out-of-sync comments in build-css.js). It would be good to clear up as much as possible of past reviews before I invest time in writing a new one, just to be sure I don't repeat myself. There also seem to be some minor eslint style issues that can be easily addressed and will hopefully reduce noise in the files tab once fixed.

demiankatz · 2026-05-06T13:37:01Z

@crhallberg, I've gone ahead and resolved merge conflicts here, though most of my outstanding review comments are still relevant; I think there's still quite a bit of work needed here. We should discuss today whether it's realistic to aim for 11.1, or if we should resign ourselves to 12.0.

demiankatz · 2026-05-06T13:38:59Z

The build is currently broken because something is trying to call installBuildDeps which has been renamed to installDeps. As discussed above, we may not want to do that. I think we may want to revisit all the changes here and see if we can reduce differences to a greater extent. Ideally I'd like to get rid of Grunt without changing the external interface by renaming NPM scripts -- ESPECIALLY if we're aiming for release 11.1.

demiankatz

Thanks, @crhallberg, this is looking a lot cleaner now... see below for a comment about further simplification/separation, though.

I've also taken care of resolving conflicts introduced by merging your language check tool. I think I incorporated workspace functionality correctly, but please double-check my work.

I've also cleaned up a few eslint indentation warnings to reduce noise in the PR.

Unfortunately, the CSS compilation isn't actually working for me. I'm getting this:

bootstrap5
- compiling SCSS to CSS...
sass.Exception [Error]: Can't find stylesheet to import.
   ╷
14 │ @import "vendor/bootstrap/scss/functions";
   │         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   ╵
  themes/bootstrap5/scss/bootstrap.scss 14:9  @import
  themes/bootstrap5/scss/compiled.scss 1:9    root stylesheet
    at Object.wrapException (/usr/local/vufind/node_modules/sass/sass.dart.js:2166:43)
    at _EvaluateVisitor1._evaluate0$_loadStylesheet$4$baseUrl$forImport (/usr/local/vufind/node_modules/sass/sass.dart.js:94280:19)
    at _EvaluateVisitor1._evaluate0$_loadStylesheet$3$forImport (/usr/local/vufind/node_modules/sass/sass.dart.js:94309:19)
    at _EvaluateVisitor__visitDynamicImport_closure1.call$0 (/usr/local/vufind/node_modules/sass/sass.dart.js:96589:17)
    at _EvaluateVisitor1._evaluate0$_withStackFrame$1$3 (/usr/local/vufind/node_modules/sass/sass.dart.js:95729:25)
    at _EvaluateVisitor1._evaluate0$_withStackFrame$3 (/usr/local/vufind/node_modules/sass/sass.dart.js:95735:19)
    at _EvaluateVisitor1._evaluate0$_visitDynamicImport$1 (/usr/local/vufind/node_modules/sass/sass.dart.js:94227:19)
    at _EvaluateVisitor1.visitImportRule$1 (/usr/local/vufind/node_modules/sass/sass.dart.js:94201:17)
    at ImportRule0.accept$1$1 (/usr/local/vufind/node_modules/sass/sass.dart.js:100089:22)
    at ImportRule0.accept$1 (/usr/local/vufind/node_modules/sass/sass.dart.js:100092:19)

Is it working for you?

demiankatz · 2026-05-12T16:04:00Z

 # Vim temporary files
 .*.sw[po]

+# local config


Might make sense to move this reorganization of .gitignore to a separate PR, and just focus on removing the obsolete lines in this one. I think it's useful, but it obscures what is actually changing here. I also think some of the lines currently under the "QA files" comment really belong elsewhere -- but I don't want to distract from the work at hand sorting all that out here. :-)

demiankatz · 2026-05-12T16:17:40Z

I've also resolved some comment threads for issues that have been addressed. However, there are still a couple of outstanding questions about comments in build-css.js (from my September review) that I believe still need attention.

This reverts commit 077f5ea.

feat: tools/build-css.js

b455e5d

crhallberg added this to the 11.0 milestone Sep 5, 2025

crhallberg self-assigned this Sep 5, 2025

crhallberg mentioned this pull request Sep 5, 2025

Replace grunt with NPM scripts (see #4591) #2624

Closed

4 tasks

crhallberg added 3 commits September 5, 2025 15:16

feat: no dependency warnings.

fc6b753

remove: grunt

34bae4d

tools: improve --help

701b109

crhallberg changed the title ~~Replace grunt with npm scripts (post bs5)~~ Replace grunt with npm scripts (post BS5) Sep 5, 2025

tools: add --check-only. improve arguments and styles.

d952f67

demiankatz requested changes Sep 6, 2025

View reviewed changes

tools: check only now doesn't run minify by default.

8067ca4

demiankatz requested changes Sep 9, 2025

View reviewed changes

crhallberg added 3 commits September 9, 2025 16:22

ignore: add theme vendor, organize.

077f5ea

tools: use npm workspaces to handle installDeps and updateDeps.

00035b1

Merge remote-tracking branch 'origin/dev' into v11-npm-scripts

aa75835

crhallberg added 2 commits September 9, 2025 16:37

fix: try catch in copyDependencies.

4fc37ef

chore: qa-js-and-scss

ef4e7f4

demiankatz requested changes Sep 10, 2025

View reviewed changes

demiankatz mentioned this pull request Sep 12, 2025

Feature: Icon Mixins #4031

Open

7 tasks

tools: split build deps back out. Revert gitignore.

47f7db3

demiankatz modified the milestones: 11.0, 11.1 Nov 4, 2025

crhallberg added 3 commits December 2, 2025 15:00

feat: undo new dir

8b199a3

feat: clearer time output

d974d2f

fix: proper folder for bootstrap import

b910754

demiankatz requested changes Dec 3, 2025

View reviewed changes

demiankatz mentioned this pull request Mar 23, 2026

Minor node.js dependency updates. #5161

Merged

demiankatz mentioned this pull request Apr 12, 2026

Fix npm security vulnerabilities: lodash, minimatch overrides + sass update #5205

Open

Merge branch 'dev' into v11-npm-scripts

42931a3

crhallberg and others added 11 commits May 7, 2026 12:29

fix: white space, docs, and deprecation in package.json

a136e47

chore: npm lock (for comparison)

2a63593

style: reduce package-lock diffs

0f1217f

style: package.json indent

14721dd

style: reorder package.json (diff reduction)

b95905e

style: scss install before compile (diff reduction)

9d8dc21

style: rename updateDeps

ab2c354

style: explain missing configs from reference CLI

0108ab4

Merge branch 'dev' into v11-npm-scripts

1532a3f

Merge remote-tracking branch 'origin/dev' into v11-npm-scripts

9bfeea3

Make eslint happy.

124f548

demiankatz requested changes May 12, 2026

View reviewed changes

crhallberg added 2 commits May 12, 2026 13:37

Revert "ignore: add theme vendor, organize."

2590aff

This reverts commit 077f5ea.

fix: even less diff

9086e4e

Conversation

crhallberg commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

From #2624

Uh oh!

demiankatz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

demiankatz Sep 6, 2025

Choose a reason for hiding this comment

Uh oh!

crhallberg Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

demiankatz Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

demiankatz left a comment

Choose a reason for hiding this comment

Uh oh!

demiankatz Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

demiankatz Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

crhallberg commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

demiankatz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

demiankatz Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

demiankatz Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

demiankatz left a comment

Choose a reason for hiding this comment

Uh oh!

demiankatz commented May 6, 2026

Uh oh!

demiankatz commented May 6, 2026

Uh oh!

demiankatz left a comment

Choose a reason for hiding this comment

Uh oh!

demiankatz May 12, 2026

Choose a reason for hiding this comment

Uh oh!

demiankatz commented May 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

crhallberg commented Sep 5, 2025 •

edited

Loading

crhallberg Sep 8, 2025 •

edited

Loading

crhallberg commented Sep 9, 2025 •

edited

Loading