Skip to content

Commit

Permalink
Fix issue with lockdown.js npm preinstall hook.
Browse files Browse the repository at this point in the history
lockdown.js uses some magic so that it gets run automatically whenever
somebody runs `npm install`.
This is done by registering a preinstall hook with npm.
lockdown.js expects this hook to be called before `npm install` installs
the packages and in fact before npm has even fetched any information
about the packages from the registry.
This used to be the case in older versions of npm but the behavior of
npm has changed and now the preinstall hook is run later.
This is too late for lockdown.js to intercept the fetches from the
registry which leaves things in a broken state.
Fixing this seems difficult but luckily for our use case we don't need
to run `npm install` interactively and don't care whether that works or
or not.
Instead we can just call lockdown directly instead of calling
`npm install`.
This allows things to work as expected.

See npm/cli#2660 but note that the exact
behavior has changed several times between versions.
  • Loading branch information
abbeyj committed Jun 5, 2023
1 parent 7d7a9eb commit 331cdfe
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
4 changes: 2 additions & 2 deletions dxr/plugins/js/makefile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
build: .npm_installed
cd analyze_js && npm install
cd analyze_js && ./node_modules/.bin/lockdown

# Remove anything within node_modules that's not checked into git. Skip things
# with spaces in them, lest xargs screw up and delete the wrong thing.
Expand All @@ -14,7 +14,7 @@ lint: build
# target redoes the install if the packages or lockdown files are newer than
# that file:
.npm_installed: analyze_js/package.json analyze_js/lockdown.json
cd analyze_js && npm install
cd analyze_js && ./node_modules/.bin/lockdown
touch $@

.PHONY: build clean lint
2 changes: 1 addition & 1 deletion makefile
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ dxr/static_unhashed/js/templates.js: dxr/templates/nunjucks/*.html \
# target redoes the install if the packages or lockdown files are newer than
# that file:
.npm_installed: tooling/node/package.json tooling/node/lockdown.json
cd tooling/node && npm install
cd tooling/node && ./node_modules/.bin/lockdown
touch $@

# Install requirements in current virtualenv:
Expand Down

0 comments on commit 331cdfe

Please sign in to comment.