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.

Sign up for GitHub

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

Jump to bottom

[Bug]: @carbon/react/icons module does not provide IDE import auto-complete #11317

Closed
2 tasks done
jharvey10 opened this issue Apr 29, 2022 · 23 comments · Fixed by #14714
Closed
2 tasks done

[Bug]: @carbon/react/icons module does not provide IDE import auto-complete #11317

jharvey10 opened this issue Apr 29, 2022 · 23 comments · Fixed by #14714
Labels
package: @carbon/react @carbon/react role: dev 🤖 severity: 3 https://ibm.biz/carbon-severity type: bug 🐛 version: 11 Issues pertaining to Carbon v11

Comments

@jharvey10
Copy link
Contributor

Package

@carbon/react

Browser

No response

Package version

1.1.0

React version

not specific to a version of react

Description

When importing icons via the @carbon/react/icons re-export, no auto-completions are provided in VS Code:

image

versus @carbon/icons-react which looks like:

image

It doesn't look like the IDE auto-completion is smart enough to follow the combination of Object.keys and Object.defineProperty to provide the exhaustive list of icons that are available at runtime/build time.

image

Though I'm not sure exactly how to accomplish it, it seems like a more direct re-export of the icons from the required module would be what is needed to get past this.

There's also an info message that pops up in vscode for this import which is not present on the icons-react version:

image

versus icons-react which has a full-blown d.ts file to which it links that provides type info:

image

CodeSandbox example

n/a

Steps to reproduce

  1. Install @carbon/react into a React project
  2. Open a component file in an IDE like VS Code
  3. Attempt to import an icon from @carbon/react/icons
  4. Observe that the auto-complete list does not show any of the available icons

Code of Conduct
@joshblack joshblack mentioned this issue Apr 29, 2022
Merged
@joshblack
Copy link
Contributor

@jdharvey-ibm trying a quick change over in: #11318 that matches what other icon libraries with sub-folder imports do just in case it's a quickfix.

Otherwise, we may need to just generate types for this folder to help out since what VSCode is doing is downloading the types from @types/carbon__icons-react from DT

@joshblack joshblack self-assigned this Apr 29, 2022
@joshblack joshblack added the package: @carbon/react @carbon/react label Apr 29, 2022
@joshblack joshblack moved this to 🏗 In Progress in Design System Apr 29, 2022
@jharvey10
Copy link
Contributor Author

@joshblack sweet! What can I do to help out with testing?

@joshblack
Copy link
Contributor

@jdharvey-ibm something that would help out a ton when it's merged in would be to:

  • Pull down latest updates from main
  • Run a build (can use yarn lerna run build --scope='@carbon/icons-react' --include-dependencies to target just the icons package)
  • Link the package with npm link to connect it to your project
  • See if autocomplete works as expected or if it's still messed up

@tay1orjones tay1orjones added severity: 3 https://ibm.biz/carbon-severity impact: high 😱 and removed impact: high 😱 labels May 2, 2022
@joshblack
Copy link
Contributor

@jdharvey-ibm if you have a sec to test out latest, let me know! Should be available now

@jharvey10
Copy link
Contributor Author

Testing it now!

@jharvey10
Copy link
Contributor Author

If I explicitly reference the index.esm file, I get autocomplete:

image

But if I reference @carbon/react/icons, I don't get the auto-complete anymore:

image

If I switch the package.json file in @carbon/react/icons to have "main": "index.esm.js", the autocomplete works, but with it set to index.js, it doesn't look like the auto-complete is smart enough to know that it should look at module instead of main and just always looks at main.

@joshblack
Copy link
Contributor

@jdharvey-ibm thanks for trying it out! Super interesting 🤔

I assume you don't run into problems with packages like react-icons with its subpaths, correct?

@joshblack
Copy link
Contributor

I wonder if we should try having the default index.js file and then only set module in package.json similar to what they do 🤔

@joshblack joshblack mentioned this issue May 9, 2022
Merged
@jharvey10
Copy link
Contributor Author

When trying the example from their docs, I do indeed get autocomplete, but it looks like they're repo must be broken down into some different folders:

image

Interestingly, I just realized that if I switch to using @carbon/react/icons/es as my import, that does work as far as autocomplete goes:

image

@joshblack
Copy link
Contributor

Looks like in the latest update this is still an issue 😞 it seems unable to resolve any kind of type file for that entrypoint.

Similarly, it seems like the default types for the package are incorrect on DefinitelyTyped which is causing some of the oddity for imports.

It seems like our only way to address this is to offer an index.d.ts file that either directly exports the types or explicitly re-exports from the @carbon/icons-react (which also needs to be updated)

@joshblack joshblack mentioned this issue Jun 2, 2022
Closed
@TannerS
Copy link
Contributor

TannerS commented Jun 16, 2022

Any update?

@joshblack
Copy link
Contributor

Hey @TannerS! 👋 It seems like the big thing for incorrect imports is that there is a local cache of packages which are probably being pulled from for completion.

In the case of @jdharvey-ibm above, it seemed like a local cache was being used:
Screen Shot 2022-06-16 at 4 08 52 PM

Deleting that cache entry seemed to fix the issue of "invalid imports" but I would love to hear from you if this helps in the IntelliJ use-case 👀

@TannerS
Copy link
Contributor

TannerS commented Jun 16, 2022 via email

@joshblack
Copy link
Contributor

@TannerS it should require no updates to the package itself, it seems like the behavior comes from the TypeScript cache versus the package

@TannerS
Copy link
Contributor

TannerS commented Jun 16, 2022 via email

@jharvey10
Copy link
Contributor Author

For me on Mac, when I stepped into the definitions file it was pulling from, it was in the node modules cache in my typescipt 4.7 directory (~/Library/Caches/typescript/4.7/node_modules/@types/carbon__icons-react). I'm guessing it's an IDE feature to attempt to download types for packages from the @types namespace in an effort to be helpful. I'm not aware of a way to turn that off for VS Code (or other editors)

As a stopgap, what I might do is pull down the type definitions from over in #11533 into a local folder in my repo and include a typeRoots property in our tsconfig file to search there first.

This is definitely being actively worked on though! @joshblack and I have had some discussions about it.

@joshblack Is this enough of an issue to where we should look into deleting the definitions from DefinitelyTyped?

@joshblack
Copy link
Contributor

@jdharvey-ibm good question, I think most downloads are still coming from pre-v11 and so would be helpful for teams wanting to use TS + pre-v11 icons. TypeScript support is definitely one of the biggest blocker for folks moving to v11, though, so I feel kind of stuck with what to do.

Do we know if there is a way for the types on DefinitelyTyped to only correspond to pre-v11 versions?

@joshblack joshblack removed their assignment Jul 19, 2022
@metonym
Copy link
Collaborator

metonym commented Aug 5, 2022 •
edited
Loading

@types/carbon__icons-react has been updated to v11.5.0.

You should now see the correct types for v11 icons.

@types:carbon__icons-react@11 5 0

@jharvey10
Copy link
Contributor Author

Confirmed that when I import via @carbon/react/icons/index.esm, I get the right set of imports available, which is awesome.

The main thing still outstanding for this issue is that I don't get the auto-complete from the standard @carbon/react/icons import; I have to use the more-specific index.esm one. Not a huge deal, but wanted to note it.

@metonym
Copy link
Collaborator

metonym commented Aug 5, 2022 •
edited
Loading

I don't get the auto-complete from the standard @carbon/react/icons import

Is that something that #11533 would solve?

Ideally, types/JSDocs would be shipped with @carbon/icons-react (see comments) so that @types/carbon__icons-react could be removed altogether.

Because even as I type this comment, @types/carbon__icons-react is already one minor version behind @carbon/icons-react, which is now v11.6.0.

@jharvey10
Copy link
Contributor Author

@metonym unfortunately for my use case I don't believe that would help. The issue I have when working with the icons through the @carbon/react export is that the index.js file at @carbon/react/icons/index.js does not appear to be defined in a way in which VS Code can compute the available imports for the icons package. Something about the cjs syntax in it and the use of defineProperty isn't properly recognized. index.esm.js works fine though since the only contents of that file are export * from '@carbon/icons-react';

@metonym
Copy link
Collaborator

metonym commented Aug 7, 2022

@jdharvey-ibm Follow-up thought: why not import icons from @carbon/icons-react directly since it's a direct dependency of @carbon/react?

@jharvey10
Copy link
Contributor Author

@metonym that's definitely doable! As a general strategy though, we want the @carbon/react package to be a one-stop shop for everything you need to develop with React + Carbon. As a workaround, that and using the es import will both do the trick; it's just not the intended behavior.

@tay1orjones tay1orjones moved this from 🏗 In Progress to ⏱ Backlog in Design System Aug 17, 2022
@metonym metonym mentioned this issue Aug 30, 2022
Closed
@sstrubberg sstrubberg added adopter: PAL Work-stream that directly helps a Pattern & Asset Library. adopter: DSAG version: 11 Issues pertaining to Carbon v11 and removed affects: developers labels Nov 16, 2022
@tay1orjones tay1orjones mentioned this issue May 24, 2023
Merged
@sstrubberg sstrubberg added role: dev 🤖 and removed adopter: PAL Work-stream that directly helps a Pattern & Asset Library. adopter: DSAG labels Sep 25, 2023
@lewandom lewandom mentioned this issue Sep 26, 2023
Merged
@github-project-automation github-project-automation bot moved this from ⏱ Backlog to ✅ Done in Design System Oct 20, 2023
kennylam pushed a commit to kennylam/carbon that referenced this issue Jul 30, 2024
@m4olivei
chore(file-uploader): update file-uploader to sb v7 (carbon-design-sy…
aa53aac 
…stem#11317)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
package: @carbon/react @carbon/react role: dev 🤖 severity: 3 https://ibm.biz/carbon-severity type: bug 🐛 version: 11 Issues pertaining to Carbon v11
Projects
Design System
Archived in project
Milestone
No milestone
6 participants
@tay1orjones @joshblack @TannerS @metonym @sstrubberg @jharvey10