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

Some icons json files empty when added to the calcite-design-system package on Windows #9938

Closed
geospatialem opened this issue Aug 1, 2024 · 3 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. estimate - 5 A few days of work, definitely requires updates to tests. p - high Issue should be addressed in the current milestone, impacts component or core functionality tooling Issues relating to build system fixes or improvements.

Comments

@geospatialem
Copy link
Member

Priority impact

p - high

Summary

Since #9255 Windows is not copying over the icons effectively, some are showing as blank json files. While the files are being created, they are, in some cases, empty to the following folders:

  • dist/calcite/assets/icon
  • src/components/icon/assets/icon
  • www/build/assets/icon

The result is the demo pages only partially load and not all components and styles are depicted on the demo pages.

image

image

image

Desired Outcome

All demo pages and icons are loaded effectively

image

Resources

No response

@geospatialem geospatialem added tooling Issues relating to build system fixes or improvements. p - high Issue should be addressed in the current milestone, impacts component or core functionality 1 - assigned Issues that are assigned to a sprint and a team member. estimate - 5 A few days of work, definitely requires updates to tests. labels Aug 1, 2024
@geospatialem geospatialem added this to the 2024-08-27 - Aug Release milestone Aug 1, 2024
@benelan benelan added 2 - in development Issues that are actively being worked on. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Aug 2, 2024
benelan added a commit that referenced this issue Aug 2, 2024
**Related Issue:** #9938

## Summary

Running `npm start` from the root of the monorepo will run `npm start`
for all of the packages that have a `start` script. Therefore, `npm
start` will launch the icon browser in addition to the component demos,
now that the `@esri/calcite-ui-components` lives in the monorepo.

This seems to cause timing issues on Windows where the icons are copied
to the components before they are all built.

To only launch the component demos, use `npm run start:components`.
@benelan benelan added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Aug 2, 2024
@github-actions github-actions bot assigned geospatialem and DitwanP and unassigned benelan Aug 2, 2024
Copy link
Contributor

github-actions bot commented Aug 2, 2024

Installed and assigned for verification.

@geospatialem geospatialem added 2 - in development Issues that are actively being worked on. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Aug 2, 2024
@geospatialem geospatialem assigned benelan and unassigned geospatialem and DitwanP Aug 2, 2024
benelan added a commit that referenced this issue Aug 12, 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](https://github.com/Esri/calcite-design-system/blob/21114caa128b9a3175c7fc21e48f3b4b94c16f58/packages/calcite-ui-icons/package.json#L25-L27).
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.
jcfranco pushed a commit that referenced this issue 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.
@geospatialem geospatialem added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Aug 17, 2024
Copy link
Contributor

Installed and assigned for verification.

@geospatialem geospatialem added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Aug 17, 2024
@geospatialem
Copy link
Member Author

#10100 did the trick - verified on Windows! 🎉🎉

The following are the commands used to verify:

  1. npm install
  2. npm run build
  3. npm --workspace="packages/calcite-components" run start

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. estimate - 5 A few days of work, definitely requires updates to tests. p - high Issue should be addressed in the current milestone, impacts component or core functionality tooling Issues relating to build system fixes or improvements.
Projects
None yet
Development

No branches or pull requests

3 participants