Skip to content

Conversation

@aoberoi
Copy link
Contributor

@aoberoi aoberoi commented Dec 6, 2025

Description

Just trying to chip away at some of the warnings on npm install in this project. Hoping to make it a bit less noisy/scary and tidy up.

  1. Resolve npm install warning for @types/diff deprecation. Just removed the package. - Related: Fix timeouts for tests and add a test summary to CI jobs #1828

    npm warn deprecated @types/[email protected]: This is a stub types definition. diff provides its own type definitions, so you do not need this installed.

  2. Migrates away from unmaintained npm package fantasticon, replaces with @twbs/fantasticon fork, which is maintained.
    • Tested by running npm run compile-icons (which is indirectly included in the postinstall script too), and then previewing the built font (assets/icons/icon-font.woff) on https://fontdrop.info/. The 3 expected glyphs were present at the expected codepoints.
    • This does NOT resolve the npm install warnings I initially was pursuing. The transitive deps of @twbs/fantasticon still generate several warnings

      npm warn deprecated @npmcli/[email protected]: This functionality has been moved to @npmcli/fs
      npm warn deprecated [email protected]: This package is no longer supported.
      npm warn deprecated [email protected]: This package is no longer supported.
      npm warn deprecated [email protected]: Glob versions prior to v9 are no longer supported
      npm warn deprecated [email protected]: Glob versions prior to v9 are no longer supported
      npm warn deprecated [email protected]: This package is no longer supported.
      npm warn deprecated @xmldom/[email protected]: this version is no longer supported, please update to at least 0.8.*

    • A few possibilities to resolve the warnings above:
      1. Land Migrates from ttf2woff2 to wawoff2 dependency twbs/fantasticon#22, which will cut off the transitive chain of older deps.
      2. (Preferred) Enhance the VSCode Extension API to explicitly allow SVGs for the Icon Contribution point. Then simplify the scripts/compile_icons.ts so that it no longer needs to use @twbs/fantasticon. I could propose this to see how far it gets.

There's some other noise in the logs, but I wanted to gauge interest on potential changes before I worked on them:

  1. Look into updating the revision of swift-docc-render, and/or eliminating patches, and/or simplifying scripts/update_swift_docc_render.ts. The scariest of warnings comes from the steps in this script:

    49 vulnerabilities (10 low, 23 moderate, 14 high, 2 critical)

  2. Try using bin/build_docs within the docs-render repo instead of vue-cli-service (aka@vue/cli-service), which is no longer maintained.
  3. Investigate simplifying the husky pre-commit hook. Two things jump out:
    • Another npm install is occurring inside the npm install (top level install triggers prepare hook, pre-commit runs, which triggers another install most likely in npx lint-staged).
    • lint-staged has an option called --fail-on-changes that the script seems to be recreating.

Tasks

  • Required tests have been written
  • Documentation has been updated
  • Added an entry to CHANGELOG.md if applicable

> npm warn deprecated @types/[email protected]: This is a stub types definition. diff provides its own type definitions, so you do not need this installed.

Related: swiftlang#1828
Replaces with @twbs/fantasticon fork, which is maintained.

Tested by running `npm run compile-icons` (which is indirectly
included in the `postinstall` script too), and then previewing
the built font (assets/icons/icon-font.woff) on
https://fontdrop.info. The 3 expected glyphs were present at
the expected codepoints.

This does NOT resolve the npm install warning I initially was
pursuring. The transitive dep chain is deep, and @twbs/fantasticon
still contains the deprecated version of @npmcli/move-file.

> npm warn deprecated @npmcli/[email protected]: This functionality has been moved to @npmcli/fs
@aoberoi aoberoi changed the title nom install cleanup npm install cleanup Dec 6, 2025
@aoberoi aoberoi changed the title npm install cleanup [draft] npm install cleanup Dec 6, 2025
@plemarquand plemarquand requested review from Copilot and removed request for Copilot December 6, 2025 13:25
@aoberoi aoberoi marked this pull request as ready for review December 6, 2025 18:42
@aoberoi aoberoi changed the title [draft] npm install cleanup npm install cleanup Dec 6, 2025
@aoberoi
Copy link
Contributor Author

aoberoi commented Dec 7, 2025

Unfortunately the failed CI test is inscrutable to me. Took a look at the logs but I can't determine which test failed, much less why it failed. Possibly a known flaky test?

Copy link
Contributor

@plemarquand plemarquand left a comment

Choose a reason for hiding this comment

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

Seems like the failure was a one-off. I rekicked the failing build and it passed. Thanks for cleaning this up!

@plemarquand plemarquand merged commit 7d9309d into swiftlang:main Dec 8, 2025
82 of 84 checks passed
@matthewbastien
Copy link
Member

matthewbastien commented Dec 8, 2025

There's some other noise in the logs, but I wanted to gauge interest on potential changes before I worked on them:

  1. Look into updating the revision of swift-docc-render, and/or eliminating patches, and/or simplifying scripts/update_swift_docc_render.ts. The scariest of warnings comes from the steps in this script:
    49 vulnerabilities (10 low, 23 moderate, 14 high, 2 critical)

  2. Try using bin/build_docs within the docs-render repo instead of vue-cli-service (aka@vue/cli-service), which is no longer maintained.

  3. Investigate simplifying the husky pre-commit hook. Two things jump out:
    Another npm install is occurring inside the npm install (top level install triggers prepare hook, pre-commit runs, which triggers another install most likely in npx lint-staged).
    lint-staged has an option called --fail-on-changes that the script seems to be recreating.

Feel free to explore these options and put up patches! I don't have any problems with cleaning these up. The only potential pitfall that comes to mind is that it may be difficult to upstream some of the patches we have to swift-docc-render as they are VS Code specific.

I do have a PR (#1964) up for updating the revision of swift-docc-render already. Though I'm working through some build failures on Windows before it can be merged.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants