Pictogrammers / pictogrammers.com

Code for Pictogrammers.com
https://pictogrammers.com
80 stars 18 forks source link

Fix Icon import in React #68

Closed HugoGresse closed 1 year ago

HugoGresse commented 1 year ago

Proposed Changes

This PR fix a bug happing in production build where the <Icon /> is React was not imported correctly resulting in this error:

Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: object.

Source: https://reactjs.org/docs/error-decoder.html/?invariant=130&args[]=object&args[]=

Types of Changes

What types of changes does your code introduce?

Checklist

Additional Information

Linked issue and PR:

mririgoyen commented 1 year ago

I don't think this is a valid change. 🤔

Are you personally experiencing issues importing the Icon component in the way our documentation states to do so?

import Icon from '@mdi/react';

Icon is the default export from that package. We use that exact import statement all over this site without any issues. Are you by chance disabling default imports? Or have some other non-standard configuration that could be causing issues with imports? Could you give a little more details into this for reproduction?

HugoGresse commented 1 year ago

I've setup a sample repo here: https://github.com/HugoGresse/material-icon-vite-import-bug

Deployed here: https://hugogresse.github.io/material-icon-vite-import-bug/

I've not digged down into it but the sample only use Vite + TS + mdi package which should be a lot of project I guess. No specific setup for vite except the base path for the github pages, directly raw project.

Expected result: some text and a red icon rotating below: ezgif-1-4233c9725f

Templarian commented 1 year ago

https://github.com/Templarian/MaterialDesign-React/blob/master/dist/Icon.d.ts#L5

Looking at the source code for the dist folder for the @mdi/react package. The default is Icon.

https://github.com/Templarian/MaterialDesign-React-Demo/blob/master/src/index.js <- The demo site for instance.

I haven't looked closely at repo that @HugoGresse put up, but my guess it's a configuration issue.

mririgoyen commented 1 year ago

I've setup a sample repo here: https://github.com/HugoGresse/material-icon-vite-import-bug

I've pulled this repo down, installed, and npm run dev and I see the icon as in your screenshot as you would expect. It appears this only happens in a Vite production build.

A quick Google search immediately led me to https://github.com/vitejs/vite/issues/4704, which is closed as a duplicate of https://github.com/vitejs/vite/issues/2139, showing that this is a configuration problem with Vite. It seems someone has a workaround on this comment.

Since this isn't a problem on our side of things, I'm going to close this and the other related PRs and issues. Best of luck figuring out things with Vite!

HugoGresse commented 1 year ago

thanks for digging into this, sorry for the noise! Crazy this behavior is still here after 2y.