This sounds very much like https://github.com/adazzle/react-data-grid/issues/1999. Is there any of the explored direction that catches your eye?
yep, we have different root cause but indeed we're going in the same direction.
I believe solution no 1 that you suggested is as simple and fast as possible. I'm opting for that.
import Clear from '@material-ui/icons/Clear';
function ClearIcon() {
return <svg viewBox="0 0 24 24" style={someStyles}>{Clear.path}</svg>;
}
@aplaskota I'm not sure how we can implement it effectively. It seems that we would need to split the source. Maybe
import clear from '@material-ui/icons/md/Clear.path';
function ClearIcon() {
return <svg viewBox="0 0 24 24" style={someStyles}>{clear}</svg>;
}
?
@oliviertassinari I think we can hold on to waiting for more followers of that idea. It seems that I'm the only one who requested that change. Actually I can copy and paste some icons into my internal component
what about making the path exported var and default export leave as it is?
in this case refactoring only affects the internals, everything else stays the same. Since the core is already in the peerDeps this shouldn't be a big problem. Time consuming yes, but not complex.
@woss So you are advocating for, using one icon as an example:
diff --git a/packages/material-ui-icons/src/Info.js b/packages/material-ui-icons/src/Info.js
index 27b35d5bf2..c6e6f2d7d8 100644
--- a/packages/material-ui-icons/src/Info.js
+++ b/packages/material-ui-icons/src/Info.js
@@ -1,6 +1,6 @@
import * as React from 'react';
import createSvgIcon from './utils/createSvgIcon';
-export default createSvgIcon(
- <path d="M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm1 15h-2v-6h2v6zm0-8h-2V7h2v2z" />
-, 'Info');
+export const path = <path d="M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm1 15h-2v-6h2v6zm0-8h-2V7h2v2z" />;
+
+export default createSvgIcon(path, 'Info');
The implementation should be as simple as:
diff --git a/packages/material-ui-icons/templateSvgIcon.js b/packages/material-ui-icons/templateSvgIcon.js
index 40232b111f..e87a42305d 100644
--- a/packages/material-ui-icons/templateSvgIcon.js
+++ b/packages/material-ui-icons/templateSvgIcon.js
@@ -1,6 +1,6 @@
import * as React from 'react';
import createSvgIcon from './utils/createSvgIcon';
-export default createSvgIcon(
- {{{paths}}}
-, '{{componentName}}');
+export const path = {{{paths}}};
+
+export default createSvgIcon(path, '{{componentName}}');
If this allows tree shaking, I think that it's worth trying out.
If it doesn't, solving the problem with:
diff --git a/packages/material-ui/src/utils/createSvgIcon.js b/packages/material-ui/src/utils/createSvgIcon.js
index 2d146ab87d..dc805367f4 100644
--- a/packages/material-ui/src/utils/createSvgIcon.js
+++ b/packages/material-ui/src/utils/createSvgIcon.js
@@ -17,6 +17,8 @@ export default function createSvgIcon(path, displayName) {
Component.displayName = `${displayName}Icon`;
}
+ Component.path = path;
+
Component.muiName = SvgIcon.muiName;
return React.memo(React.forwardRef(Component));
could be even simpler.
I have tried to advance the reflection from https://github.com/adazzle/react-data-grid/issues/1999. From what I understand, we have these options:
@material-ui/core for nothing, especially for a library. But to be fair a library would probably not install a full list of 5,000 icons for using 5.@eps1lon A related question on the size of the icons repository. Do you know when would we be able to drop the .cjs extension?
I'm also wondering. Do we really need to support `import { Info } from '@material-ui/icons';
yeah, exactly that. The implementation looks simple and you don't have to create any new files. :)
As for the simpler solution, i cannot comment since i am not super familiar with the React.memo
@woss I assume by "that", you are referring to option 1. Would you mind sharing more about your use case for using only the SVG path? Thanks
@oliviertassinari yep, option 1.
as for your question i guess you would like to know how would we use the path exactly?
this is the grommet code for one icon
import { StyledIcon } from '../StyledIcon';
export const Accessibility = props => (
<StyledIcon viewBox='0 0 24 24' a11yTitle='Accessibility' {...props}>
<path fill='none' stroke='#000' strokeLinecap='round' strokeLinejoin='round' strokeWidth='2' d='M4,8 L11,8 L11,14 L7,21 M20,8 L13,8 L13,14 L17,21 M12,5 C12.5522847,5 13,4.55228475 13,4 C13,3.44771525 12.5522847,3 12,3 C11.4477153,3 11,3.44771525 11,4 C11,4.55228475 11.4477153,5 12,5 Z M11,8 L13,8 L13,13 L11,13 L11,8 Z' />
</StyledIcon>
);
So i guess we would make the Wrapper Component to use the StyledIcon similar to this where path would be a prop then use it like
<SvgIcon path={path}/>
I hope i answered your question.
@woss Would it be OK if you still have to install @material-ui/core (what option 1 implies, as far as I know)?
Do you know when would we be able to drop the .cjs extension?
As far as I remember we only use .js extensions for shipped files due to lacking tooling support. As to not shipping a commonjs build: Once we drop support for node 10. Node 10 reaches EOL on 2021-04-30.
@oliviertassinari well i guess installing no, but bundling it yes. not sure atm, i'll have to check with my guys and tell you more after that
on the other hand i got the idea to completely extract paths into the separate folder called paths inside the packages/material-ui-icons ( for the time being and for the discussion) @oliviertassinari check my fork and implementation here https://github.com/woss/material-ui/tree/woss-extract-paths/packages/material-ui-icons/paths
If we go this way, you could create separate repo just for the paths, then include that into the icons and the rest of us can just install the paths and have our implementations.
Later when google introduces new icons, or changes the current ones, just update this new repo and voil脿 we all benefit and get then new updated icons without any overhead.
Please tell me what do you think
@eps1lon Ok thanks. It sounds like we will be able to make the icons repository lighter in the near future.
@woss So you are suggesting option 2 or 3 from https://github.com/mui-org/material-ui/issues/21251#issuecomment-688923161?
With this approach option 3 would be the best, don't you think?
Option 2 with the new files that i've created would be a good solution but you would still need to do the same thing as in option 3, difference would be that with option 3 material-io/core is not installed even declared since the files are just strings and it can be used anywhere.
I understand that it requires time to set it up but honestly i've spent 30 min creating these files, mostly went on the how to sed this and that.
Option 3: when you make the new package iconPaths or however you want to call it, let me know and i will add the files from my fork to it. then we PR and merge upon the approval, you publish it and i get to install them and get rid of bloated package we are using now :)
@woss If I understand: a. you don't want to have @material-ui/core bundled, why? b. you don't want @material-ui/core to be installed in your file system, why?
Note that with option 2, we can make @material-ui/core an optional peer dependency, which will solve both a. and b.
@oliviertassinari
a: i really don't need it
b: i really don't need it
Why creating new package that will hold only the svg paths and nothing else doesn't sound like a great approach?
@woss If all you want are the raw SVG paths, you might be better off starting here (Although it seems that whoever at Google is now maintaining this scrapes them! 馃槅)