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] Improve styled-components integration #28713

Conversation

mnajdova
Copy link
Member

Fixes #28559

@mnajdova mnajdova requested a review from siriwatknp September 30, 2021 11:57
@mui-pr-bot
Copy link

mui-pr-bot commented Sep 30, 2021

No bundle size changes

Generated by 🚫 dangerJS against 45253a7

@@ -33,6 +37,39 @@ By default, `@mui/material` has `@mui/styled-engine` as a dependency, but you ca
}
```

### npm

As the package aliases do not work with npm at this moment, you need to update you bundler's config to add this alias. Here is an example of how you can do it, if you use `webpack`:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this should work from npm v6.9? https://npm.community/t/release-npm-6-9-0/5911

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't work. See #27917, we needed to revert the changes for it.

@@ -1,4 +1,5 @@
{
"extends": "./tsconfig.paths.json",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason to split out the paths to its own file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, currently tsconfig paths are not supported:

The following changes are being made to your tsconfig.json file:
  - compilerOptions.paths must not be set (aliased imports are not supported)

This is kind of an override to this. See https://stackoverflow.com/questions/57070052/create-react-app-typescript-3-5-path-alias

@oscar-b
Copy link

oscar-b commented Oct 1, 2021

Looks good but haven't been able to test it yet (quite swamped with other tasks at work) :)

Co-authored-by: Oscar Bolmsten <oscar@bolmsten.se>
@mnajdova
Copy link
Member Author

mnajdova commented Oct 1, 2021

Looks good but haven't been able to test it yet (quite swamped with other tasks at work) :)

Thanks for looking into it. I've tested it myself, but will wait for a confirmation from at least one more person :)

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 1, 2021
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 1, 2021
Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 tested.

Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Package aliases most likely do work. However, a package alias via dependencies only applies to imports in your source code. Not imports from within node_modules. But we do need to affect imports from within node_modules (@mui/*) so we need Yarn Resolutions.

Added suggestions to make the distinction between Alias and Resolution clearer.

@eps1lon eps1lon added the docs Improvements or additions to the documentation label Oct 4, 2021
mnajdova and others added 2 commits October 4, 2021 10:14
Co-authored-by: Sebastian Silbermann <silbermann.sebastian@gmail.com>
Co-authored-by: Sebastian Silbermann <silbermann.sebastian@gmail.com>
@mnajdova
Copy link
Member Author

mnajdova commented Oct 4, 2021

Added suggestions to make the distinction between Alias and Resolution clearer.

Thanks, I don't know what I kept referring to them as aliases.

@mnajdova mnajdova requested a review from eps1lon October 4, 2021 08:24
@eps1lon
Copy link
Member

eps1lon commented Oct 4, 2021

Added suggestions to make the distinction between Alias and Resolution clearer.

Thanks, I don't know what I kept referring to them as aliases.

Probably because Webpack calls them "aliases".

@mnajdova mnajdova merged commit d4ea4c9 into mui:master Oct 4, 2021
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.

[style-engine] style-components resolutions does not work with npm
5 participants