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

[docs] Link a VSCode extension for working with Material-UI #19280

Merged

Conversation

jedwards1211
Copy link
Contributor

I think VSCode users out there will really love this extension I just made!

It has commands for wrapping a component in the text editor in withStyles, and another for automatically creating/updating up the declaration of Box for work with @material-ui/system.

https://marketplace.visualstudio.com/items?itemName=vscodeshift.material-ui-codemorphs

@jedwards1211 jedwards1211 changed the title [Related Proejcts] plug my new VSCode extension [Related Proejcts] plug my new VSCode extension for working with MUI styles/system Jan 18, 2020
@mui-pr-bot
Copy link

mui-pr-bot commented Jan 18, 2020

No bundle size changes comparing 39c8170...d741869

Generated by 🚫 dangerJS against d741869

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 18, 2020

@jedwards1211 Thanks for sharing. This is a pretty exciting path to explore and raise an interesting question: How can we improve the DX, down to the code editor?

However, I think that it's too early to promote it here. I think that the best approach would be to see how it perform (as evaluated by the community, not the core team), then we can give it further exposure. A few notes:

  • Code snippets for each component would be pretty awesome. It seems to be what would fill the most pain: https://marketplace.visualstudio.com/search?term=bootstrap&target=VSCode&category=All%20categories&sortBy=Relevance. I would likely use them personally.
  • I don't understand the value of the Box's codemod.
  • The gif makes it hard to follow what's going on. A before and after static code preview could help, or slowing down the gif.
  • Nested import > 1 level deep is forbidden. Please change the import of the withStyles codemod.
  • It would be pretty cool if we could preview the colors from the API of Material-UI.

Capture d’écran 2020-01-18 à 11 31 56

  • Moving the prop definitions to TypeScript is pretty useful with autocompletion

Capture d’écran 2020-01-18 à 11 55 21

@jedwards1211
Copy link
Contributor Author

Actually someone made this for code snippets, but I haven't tried it: https://marketplace.visualstudio.com/items?itemName=Arunagiritm.material-ui
Somehow I haven't been tempted to try it because I doubt it would work well for things like adding a dialog into an existing component...

I should provide a better demo of the Box codemod...the tedious thing when using @material-ui/system is you have to figure out which style functions to import and compose together in the declaration of Box to support which props you're using. The Box codemod does that automatically for you.
I'll work on making the README clearer, there's a before and after in this subproject: https://github.com/jedwards1211/material-ui-codemorphs#example

@jedwards1211
Copy link
Contributor Author

I'm sorry but I got the import path for Theme from your own docs:
https://material-ui.com/guides/typescript/#customization-of-theme

Is there an alternative path I can import it from?
Since it's a type import I assume it doesn't really matter how deep the path is?

@jedwards1211
Copy link
Contributor Author

When using system, do you typically just stuff all the style function imports you need for the entire project in one module and then import that wherever you need it?

I figured that with code splitting some chunks might be smaller if each module that uses system imports only the style functions it needs.

@jedwards1211
Copy link
Contributor Author

@oliviertassinari okay, I fixed the import paths now, looks like there's no problem with importing Theme and WithStyles from @material-ui/core/styles. I guess we should update the docs though

@oliviertassinari
Copy link
Member

Thanks for raising the issue with the docs. I think that we need to update the TypeScript's imports.

Regarding the Box, the tree shaking win is negligible. I think that you will be better off optimizing the cost of changes.

Closing per the initial concern: let's have the community to weight in.

@jedwards1211
Copy link
Contributor Author

jedwards1211 commented Jan 19, 2020

What do you mean by optimizing the cost of changes?

@oliviertassinari
Copy link
Member

What happens if you want to use a new prop that isn't available? You will need to rerun the codemod.

@jedwards1211
Copy link
Contributor Author

jedwards1211 commented Jan 19, 2020

Right, but no one starts off by using every single style function anyway, do they? Running the codemod takes less than a second, you can even assign a keyboard shortcut to it. I think you overestimate the effort required to use it. The extension doesn't run jscodeshift on your entire project, it just runs it on the current text editor.

@jedwards1211
Copy link
Contributor Author

Try the Box codemod out and see how it feels to actually use it. It frees you from ever looking up which style function goes with which properties ever again, which is something you'd still have to do once in awhile if you compose all the style functions together in a single place in your project.

@jedwards1211
Copy link
Contributor Author

I don't think the community will actually be able to discover this and try it out unless it's publicized somehow...would it be too much to ask you to mention it on Twitter if I can't get a link in the related projects?

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 19, 2020

Right, but no one starts off by using every single style function anyway, do they?

I think that most do, at least when they use the Box. I have personally never imported anything from @material-ui/system in a project.

I don't think the community will actually be able to discover this and try it out unless it's publicized somehow.

The Bootstrap community was able to discover the VS code extensions without linking it to the documentation. Same thing for Vuetify. For a UI library, I think that developers will be most interested in snippets. However, it might inspire others, so why not!

@oliviertassinari oliviertassinari changed the title [Related Proejcts] plug my new VSCode extension for working with MUI styles/system [docs] Link a VSCode extension for working with Material-UI Jan 19, 2020
@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Jan 19, 2020
@oliviertassinari
Copy link
Member

Regarding the withStyles codemod, it's pretty cool. Every time I have to add styles to a component, I feel friction, no more :).

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 19, 2020

@jedwards1211 What about the following action plan? :)

@oliviertassinari oliviertassinari added the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Jan 19, 2020
@jedwards1211
Copy link
Contributor Author

That sounds good, I can work on that in the next few days

@jedwards1211
Copy link
Contributor Author

jedwards1211 commented Jan 20, 2020

Okay looking at @tmarun's snippets I see some problems:

  • Most importantly, only JS snippets are included, anything "official" would need TS snippets too.
  • Some of the snippets create class property arrow functions for callbacks like onClose, followed directly by a JSX element - these wouldn't be very convenient to expand anywhere
  • They prefixes all start with mui-. I feel on the fence about whether this is necessary. It's kind of a shame that there's no way to configure this in a snippets extension. They should have just made a JS API for defining snippets. The limitations of JSON for defining snippets are a big mistake.

How about I make a fork that addresses these issues? I'd also like to make the project structure more maintainable. (The JSON syntax for multiline snippets is truly awful, I'd rather define the snippets in JS files and have a build task output the final JSON for VSCode to use)

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 20, 2020

@jedwards1211 Thanks for the feedback on the snippet extension. The point regarding TypeScript is interesting. It seems that the withStyles codemod fails on a JavaScript file:

Capture d’écran 2020-01-20 à 13 23 52

Regarding the prefix, I have seen Bootstrap prefix it with b4-, Vuetify with v-, Ionic with i-. Ant Design with ant. What do you have in mind?

Regarding forking, I don't think that Material-UI should officially support any extension, but if you have interests in pushing this further, sure, the better quality the extension available, the better for the community :)! Maybe you could find ways to join forces with @tmarun. No matter what, I wonder if developers would prefer different extensions for each concerns or a single one.

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 6, 2020

@jedwards1211 I hope these extentions will get adoption, keep up the good work :)

@jedwards1211
Copy link
Contributor Author

jedwards1211 commented Feb 6, 2020

Thanks!
I want to help out with more migration type codemods too, if I made a codemod to migrate from withStyles to useStyles would you accept a PR for it? Let me know if there are any other things that would be useful, I love writing codemods

@oliviertassinari
Copy link
Member

We want to use and document styled-components by default in v5. A codemod that could help with this would also be great.

@jedwards1211
Copy link
Contributor Author

jedwards1211 commented Feb 7, 2020

Oh no 😱
I'm not a fan of styled-components...being a user of JSS before I came to MUI, I was so happy that MUI uses it.
Or do you mean the plan is that either one can be used with MUI?
I wouldn't want styled-components to be required, because it would increase the bundle size of anyone who's already got a lot of JSS in their app.

@oliviertassinari
Copy link
Member

@jedwards1211 Ideally, styled-components will be optional but come by default.

@jedwards1211
Copy link
Contributor Author

Huh, what's motivating this? I don't get why any React devs would be into it. We don't like putting our HTML in template strings, so why would we want to put our CSS in template strings?

JS Objects Template Strings
HTML React ✨ Angular
CSS JSS styled-components

That said I could try to make a codemod for it. I would want to make a codemod for the reverse direction back to JSS too though. Though it would be harder because I would have to parse the CSS in the template strings -- which is an example of how template strings are less powerful to work with than JS objects.

@mbrookes
Copy link
Member

mbrookes commented Feb 7, 2020

@jedwards1211 We track it here: #6115

EsoterikStare pushed a commit to EsoterikStare/material-ui that referenced this pull request Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants