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

build(deps): clean up ui-icon dependencies #10024

Merged
merged 7 commits into from
Aug 12, 2024
Merged

build(deps): clean up ui-icon dependencies #10024

merged 7 commits into from
Aug 12, 2024

Conversation

benelan
Copy link
Member

@benelan benelan commented Aug 9, 2024

Related Issue: #9938

Summary

The @esri/calcite-ui-icons code was not maintained so a lot of the dependencies were extremely outdated. I'm hoping updating them will fix the Windows build issue.

  • Remove some @esri/calcite-ui-icons dependencies in favor of the Node standard library
  • Update most of the remaining dependencies

There are still two outdated dependencies:

  • camelcase: bumping to v7 or above will require switching to ESM, which might be a breaking change. For reference, the npm package exports ESM files from js/* for the icon SVG paths. However, the Node scripts use CJS and one of them is exposed for CLI usage. I'm not sure if anyone uses it, but I don't want to change the modules just in case.
  • svgo: There were significant changes in the new major version(s), including a big rework of the configuration. I tried bumping it on benelan/update-svgo, but diffing the JSON build files showed a lot of changes. It's possible/likely that there isn't a way to get the exact same output after bumping the major version, but I'd need a designer to look at the SVGs to make sure everything is still okay.

@benelan benelan force-pushed the benelan/icon-ci-fixes branch from fec3287 to 6faabe5 Compare August 9, 2024 07:04
@github-actions github-actions bot added chore Issues with changes that don't modify src or test files. skip visual snapshots Pull requests that do not need visual regression testing. labels Aug 9, 2024
@benelan benelan force-pushed the benelan/icon-ci-fixes branch from 6faabe5 to 271d592 Compare August 9, 2024 07:09
@benelan benelan force-pushed the benelan/icon-ci-fixes branch from 271d592 to e8fa589 Compare August 9, 2024 07:10
@benelan benelan marked this pull request as draft August 10, 2024 03:59
@benelan benelan removed the skip visual snapshots Pull requests that do not need visual regression testing. label Aug 12, 2024
@benelan benelan marked this pull request as ready for review August 12, 2024 20:27
@benelan benelan added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Aug 12, 2024
@jcfranco jcfranco changed the title build(deps): cleanup ui-icon dependencies build(deps): clean up ui-icon dependencies Aug 12, 2024
@benelan benelan merged commit cfaebcb into dev Aug 12, 2024
13 checks passed
@benelan benelan deleted the benelan/icon-ci-fixes branch August 12, 2024 21:42
@github-actions github-actions bot added this to the 2024-08-27 - Aug Release milestone Aug 12, 2024
benelan added a commit that referenced this pull request Aug 13, 2024
…to-monorepo

* origin/dev: (37 commits)
  chore: release next
  fix(tree,tree-item): replace checkbox with div and a11y attributes (#9849)
  feat(combobox-item): add `heading` property (deprecates `textLabel`) and add `label` property (#9987)
  chore: stop requesting review from ben/franco for all PRs (#10040)
  feat: add close-caption, transcript, flag, and flag-slash (#10039)
  build(deps): update dependency @cspell/eslint-plugin to v8.13.2 (#10036)
  build(deps): update angular-cli monorepo to v18.1.4 (#10035)
  build(deps): clean up ui-icon dependencies (#10024)
  build: update browserslist db (#10033)
  docs: update component READMEs (#10034)
  ci: fix chromatic and pr-semantic check trigger events (#10041)
  chore: release next
  fix(label): prevent focusing and toggling elements slotted within a label when a text selection occurs (#9990)
  build(deps): update dependency @floating-ui/dom to v1.6.9 (#10025)
  build(deps): update dependency postcss to v8.4.41 (#10027)
  ci: new version of husky no longer need npx (#10030)
  build(deps): update dependency lerna to v8.1.8 (#10026)
  build(deps): update dependency chromatic to v11.7.0 (#9995)
  build(deps): update dependency eslint-plugin-jsdoc to v48.11.0 (#9997)
  chore: release next
  ...
jcfranco pushed a commit that referenced this pull request Aug 17, 2024
**Related Issue:** #9938

## Summary

I resolved the issue linked above in #10024, but it created a new error
on Windows machines:

```text
@esri/calcite-ui-icons:build: 🚨  Error while generating icons.json
@esri/calcite-ui-icons:build: Error: EMFILE: too many open files, open 'C:\Dev\calcite-design-system\packages\calcite-ui-icons\js\cursorMinus16.json'
@esri/calcite-ui-icons:build:     at async open (node:internal/fs/promises:639:25)
@esri/calcite-ui-icons:build:     at async writeFile (node:internal/fs/promises:1219:14)
@esri/calcite-ui-icons:build:     at async Promise.all (index 8189) {
@esri/calcite-ui-icons:build:   errno: -4066,
@esri/calcite-ui-icons:build:   code: 'EMFILE',
@esri/calcite-ui-icons:build:   syscall: 'open',
@esri/calcite-ui-icons:build:   path: 'C:\\Dev\\calcite-design-system\\packages\\calcite-ui-icons\\js\\cursorMinus16.json'
@esri/calcite-ui-icons:build: }
```

This adds back `fs-extra`, which uses `graceful-fs` to prevent EMFILE
errors, as mentioned in their
[README](https://github.com/isaacs/node-graceful-fs#improvements-over-fs-module):

> Queues up open and readdir calls, and retries them once something
closes if there is an EMFILE error from too many file descriptors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Issues with changes that don't modify src or test files. pr ready for visual snapshots Adding this label will run visual snapshot testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants