Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate from deprecated VSCode dependencies #161

Merged
merged 2 commits into from
Dec 20, 2023

Conversation

nodeg
Copy link
Member

@nodeg nodeg commented May 5, 2023

🤔 What's changed?

2 dependencies in package.json and one in package-lock.json

⚡️ What's your motivation?

Get rid of:

$ npm install
npm WARN deprecated @npmcli/[email protected]: This functionality has been moved to @npmcli/fs
npm WARN deprecated [email protected]: This package has been renamed to @vscode/test-electron, please update to the new name
npm WARN deprecated [email protected]: vsce has been renamed to @vscode/vsce. Install using @vscode/vsce instead.
(...)

3 vulnerabilities (2 moderate, 1 high)

(...)
npm install  102.44s user 15.54s system 93% cpu 2:06.42 total

$ npm audit
# npm audit report

json5  <1.0.2
Severity: high
Prototype Pollution in JSON5 via Parse Method - https://github.com/advisories/GHSA-9c47-m6qq-7p4h
fix available via `npm audit fix`
node_modules/tsconfig-paths/node_modules/json5

1 high severity vulnerability

🏷️ What kind of change is this?

  • 🏦 Refactoring/debt/DX (improvement to code design, tooling, documentation etc. without changing behaviour)

♻️ Anything particular you want feedback on?

I ran the updated test and a build afterwards with success:

$ npm run test

> [email protected] pretest
> npm run compile


> [email protected] compile
> tsc --build


> [email protected] test
> node ./dist/src/test/runTest.js

Found existing install in /Users/dom/git/vscode/.vscode-test/vscode-darwin-1.78.0. Skipping download
[main 2023-05-05T15:29:29.445Z] update#ctor - updates are disabled by the environment
Via 'product.json#extensionEnabledApiProposals' extension 'ms-vscode.vscode-selfhost-test-provider' wants API proposal 'testContinuousRun' but that proposal DOES NOT EXIST. Likely, the proposal has been finalized (check 'vscode.d.ts') or was abandoned.
2023-05-05 17:29:30.458 Code Helper (Renderer)[97671:1054968] CoreText note: Client requested name ".NewYork-Regular", it will get TimesNewRomanPSMT rather than the intended font. All system UI font access should be through proper APIs such as CTFontCreateUIFontForLanguage() or +[NSFont systemFontOfSize:].
2023-05-05 17:29:30.458 Code Helper (Renderer)[97671:1054968] CoreText note: Set a breakpoint on CTFontLogSystemFontNameRequest to debug.
Loading development extension at /Users/dom/git/vscode/dist
Bad control character in string literal in JSON at position 2

  Extension Test Suite
    ✔ Sample test
  1 passing (3ms)
[main 2023-05-05T15:29:32.063Z] Extension host with pid 97702 exited with code: 0, signal: unknown.
Exit code:   0
Done

npm run test  10.38s user 2.67s system 155% cpu 8.379 total

$ npm run build

> [email protected] build
> npm run esbuild-extension


> [email protected] esbuild-extension
> esbuild ./src/extension.ts --external:vscode --bundle --outfile=out/extension.js --format=cjs --platform=node --minify --sourcemap


  out/extension.js      996.3kb
  out/extension.js.map    3.3mb

⚡ Done in 235ms

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

@kieran-ryan
Copy link
Member

Hi @nodeg, thank you very much for your contribution; apologies there has been no review on your pull request.

The warnings and vulnerabilities you observed appear to no longer be present - the Renovate configuration for the Cucumber organisation appears to be doing a good job of keeping dependencies up to date.

Would you perhaps be able clone the repository and verify if the pull request can be closed? Otherwise we can examine whether we can update and get it merged.

Some dependency names have changes. This commit changes them to the new
ones and also bumps their version.

Signed-off-by: Dominik Gedon <[email protected]>
@nodeg nodeg force-pushed the update_dependencies branch from 503da0b to 6538b2e Compare December 20, 2023 13:32
@nodeg
Copy link
Member Author

nodeg commented Dec 20, 2023

I did some more testing with the most recent changes from the main branch and still see some of the issues

$ npm install
npm WARN deprecated [email protected]: This package has been renamed to @vscode/test-electron, please update to the new name
npm WARN deprecated [email protected]: vsce has been renamed to @vscode/vsce. Install using @vscode/vsce instead.

> [email protected] prepare
> npm run copy-wasms


> [email protected] copy-wasms
> mkdir -p out && cp node_modules/@cucumber/language-service/dist/*.wasm out


added 753 packages, and audited 754 packages in 47s

200 packages are looking for funding
  run `npm fund` for details

2 moderate severity vulnerabilities

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.
$ npm audit
# npm audit report

xml2js  <0.5.0
Severity: moderate
xml2js is vulnerable to prototype pollution - https://github.com/advisories/GHSA-776f-qx25-q3cc
fix available via `npm audit fix --force`
Will install [email protected], which is a breaking change
node_modules/xml2js
  vsce  >=1.98.0-alpha.0
  Depends on vulnerable versions of xml2js
  node_modules/vsce

2 moderate severity vulnerabilities

To address all issues (including breaking changes), run:
  npm audit fix --force

After applying my changes, the issues are gone:

$ npm install

> [email protected] prepare
> npm run copy-wasms


> [email protected] copy-wasms
> mkdir -p out && cp node_modules/@cucumber/language-service/dist/*.wasm out


added 740 packages, and audited 741 packages in 53s

200 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
$ npm run test

> [email protected] pretest
> npm run compile


> [email protected] compile
> tsc --build


> [email protected] test
> node ./dist/src/test/runTest.js

Downloading VS Code 1.85.1 from https://update.code.visualstudio.com/1.85.1/darwin/stable
Downloading VS Code [==============================] 100%
Downloaded VS Code into /Users/dom/git/vscode/.vscode-test/vscode-darwin-1.85.1

Downloaded VS Code into /Users/dom/git/vscode/.vscode-test/vscode-darwin-1.85.1
2023-12-20 14:30:19.263 Electron[94335:1720694] WARNING: Secure coding is not enabled for restorable state! Enable secure coding by implementing NSApplicationDelegate.applicationSupportsSecureRestorableState: and returning YES.
[main 2023-12-20T13:30:20.199Z] update#setState disabled
[main 2023-12-20T13:30:20.201Z] update#ctor - updates are disabled by the environment
Via 'product.json#extensionEnabledApiProposals' extension 'ms-python.python' wants API proposal 'registerIssueDataProvider' but that proposal DOES NOT EXIST. Likely, the proposal has been finalized (check 'vscode.d.ts') or was abandoned.
2023-12-20 14:30:22.264 Code Helper (Renderer)[94342:1720943] CoreText note: Client requested name ".NewYork-Regular", it will get TimesNewRomanPSMT rather than the intended font. All system UI font access should be through proper APIs such as CTFontCreateUIFontForLanguage() or +[NSFont systemFontOfSize:].
2023-12-20 14:30:22.264 Code Helper (Renderer)[94342:1720943] CoreText note: Set a breakpoint on CTFontLogSystemFontNameRequest to debug.
Started local extension host with pid 94354.
Loading development extension at /Users/dom/git/vscode/dist

  Extension Test Suite
    ✔ Sample test
  1 passing (5ms)
[main 2023-12-20T13:30:24.525Z] Extension host with pid 94354 exited with code: 0, signal: unknown.
Exit code:   0
Done

I did rebase and update my PR.

@kieran-ryan kieran-ryan changed the title Rename dependencies Migrate from deprecated VSCode dependencies Dec 20, 2023
- As of VSCode 1.74, activation events can be detected based on declared 'contributes' - https://code.visualstudio.com/updates/v1_74#_implicit-activation-events-for-declared-extension-contributions. Is made possible with migration to `@vscode/vsce` dependency
- Fixes a linting error with unordered imports
Copy link
Member

@kieran-ryan kieran-ryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for your contribution @nodeg. Had to resolve a linting error - this step is now included in the contributing guidelines (#193). Also removed declaration of the activation event which is no longer required as of VSCode 1.74 and was enabled by your contribution - looking forward to your next one!

@kieran-ryan kieran-ryan merged commit caa5b3b into cucumber:main Dec 20, 2023
2 checks passed
@aslakhellesoy
Copy link
Contributor

Hi @nodeg,

Thanks for your making your first contribution to Cucumber, and welcome to the Cucumber committers team! You can now push directly to this repo and all other repos under the cucumber organization! 🍾

In return for this generous offer we hope you will:

  • ✅ Continue to use branches and pull requests. When someone on the core team approves a pull request (yours or someone else's), you're welcome to merge it yourself.
  • 💚 Commit to setting a good example by following and upholding our code of conduct in your interactions with other collaborators and users.
  • 💬 Join the community Slack channel to meet the rest of the team and make yourself at home.
  • ℹ️ Don't feel obliged to help, just do what you can if you have the time and the energy.
  • 🙋 Ask if you need anything. We're looking for feedback about how to make the project more welcoming, so please tell us!

On behalf of the Cucumber core team,
Aslak Hellesøy
Creator of Cucumber

kieran-ryan added a commit to kieran-ryan/vscode that referenced this pull request Dec 20, 2023
* Update renamed dependencies

Some dependency names have changes. This commit changes them to the new
ones and also bumps their version.

Signed-off-by: Dominik Gedon <[email protected]>

* Removed redundant activation event

- As of VSCode 1.74, activation events can be detected based on declared 'contributes' - https://code.visualstudio.com/updates/v1_74#_implicit-activation-events-for-declared-extension-contributions. Is made possible with migration to `@vscode/vsce` dependency
- Fixes a linting error with unordered imports

---------

Signed-off-by: Dominik Gedon <[email protected]>
Co-authored-by: Kieran Ryan <[email protected]>
kieran-ryan added a commit to kieran-ryan/vscode that referenced this pull request Dec 20, 2023
* Update renamed dependencies

Some dependency names have changes. This commit changes them to the new
ones and also bumps their version.

Signed-off-by: Dominik Gedon <[email protected]>

* Removed redundant activation event

- As of VSCode 1.74, activation events can be detected based on declared 'contributes' - https://code.visualstudio.com/updates/v1_74#_implicit-activation-events-for-declared-extension-contributions. Is made possible with migration to `@vscode/vsce` dependency
- Fixes a linting error with unordered imports

---------

Signed-off-by: Dominik Gedon <[email protected]>
Co-authored-by: Kieran Ryan <[email protected]>
@nodeg nodeg deleted the update_dependencies branch December 20, 2023 20:24
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.

3 participants