chore(deps): update update-notifier to 7.0.0 #1544
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sjinks
commented
Nov 6, 2023
Comment on lines
-93
to
-97
| setEnvironmentVariabeles( envVars ); | ||
| // We have to dynamically import the module so we can set environment variables above | ||
| // All tests must be async to support this dynamic import, otherwise the modified env variables are not picked up | ||
| const createProxyAgent = ( await import( '../../../src/lib/http/proxy-agent' ) ) | ||
| .createProxyAgent; |
There was a problem hiding this comment.
It magically works :-) I found no code in createProxyAgent to cache the old environment variables. And, as far as I understand, multiple await import (= require()) will load the module only once anyway.
sjinks
commented
Nov 6, 2023
Comment on lines
+30
to
+32
| afterAll( () => { | ||
| process.env = { ...env }; | ||
| } ); |
There was a problem hiding this comment.
Maybe this is unnecessary, but I prefer restoring things to their original state.
abdullah-kasim
approved these changes
Nov 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR updates
update-notifierto 7.0.0. Becauseupdate-notifieris now an ESM, this required several additional steps.update-notifieris imported viaawait import- that's the only way to import ESM from CJSbabeltries to replaceawait importwithPromise.resolve().then(() => _interopRequireWildcard(require('update-notifier')), I had to disable the@babel/plugin-proposal-dynamic-importplugin.proxy-agenttests becausejestdid not likeawait importand insisted on the ESM-enabled environment. Luckily, the test works without dynamic imports.update-notifierwhenNODE_ENV=testbecause I did not want to installcross-envand passNODE_OPTIONS=--experimental-vm-modulesto keep Jest happy. However, we may need to do this someday.As a bonus, we have four less medium security vulnerabilities.
Steps to Test
npm run build; ./dist/bin/vip.jsshould work (it will invokeupdate-notifier; there should be no errors coming from Node.js). E2E tests will also catch an error here.