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] Use immediate export when there is no HOC part 2 #16038

Merged
merged 3 commits into from
Jun 9, 2019

Conversation

merceyz
Copy link
Member

@merceyz merceyz commented Jun 3, 2019

Followup to #16005, VSCode failed to find and replace all the instances on the first try. Used formattedTSDemos to do it correctly this time

@merceyz merceyz changed the title [docs] Make formattedTSDemos fix immediate exports [docs] Make formattedTSDemos change to immediate exports Jun 3, 2019
@mui-pr-bot
Copy link

mui-pr-bot commented Jun 3, 2019

No bundle size changes comparing 396e379...36c4dd6

Generated by 🚫 dangerJS against 36c4dd6

@merceyz merceyz force-pushed the docs/exports branch 3 times, most recently from 0c3de4f to a603491 Compare June 3, 2019 22:57
@eps1lon
Copy link
Member

eps1lon commented Jun 4, 2019

I appreciate the effort but this is a lint rule in disguise and we should have a discussion about those. It is IMO wasteful to introduce lint rules into the codebase without any rationale. They add diff noise and require a conscious code style shift when working on this codebase.

Is there a rational for

-function foo() {}
-export default foo;
+ export default function foo() {}

?

@eps1lon eps1lon added the docs Improvements or additions to the documentation label Jun 4, 2019
@merceyz
Copy link
Member Author

merceyz commented Jun 4, 2019

See #16003 (comment)

@eps1lon
Copy link
Member

eps1lon commented Jun 4, 2019

So "shorten the demos". Not a fan but makes sense. Still better implemented with a lint rule. I can take care of that in another PR.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 4, 2019

I would emphasize the value on "not having to duplicate the component name". It's one less degree of freedom (so you can't use two different names by mistake). This is only marginally better, it won't change thing much at the end of the day. I believe it's one of those small details that compounds.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 9, 2019

@merceyz Can we split the effort? I propose that we remove the automatic generation part of this pull request that has controverse around it. This way, we can benefit from the documentation improvement by merging it. It will also make the review of the eslint or code generation logic easier in the next pull request.

@merceyz merceyz changed the title [docs] Make formattedTSDemos change to immediate exports [docs] Use immediate export when there is no HOC part 2 Jun 9, 2019
@merceyz
Copy link
Member Author

merceyz commented Jun 9, 2019

@oliviertassinari Sure, I ran the command again to update the remaining demos and reverted the changes to formattedTSDemos.

@oliviertassinari
Copy link
Member

Perfect!

@oliviertassinari oliviertassinari merged commit f63df21 into mui:master Jun 9, 2019
@merceyz merceyz deleted the docs/exports branch June 9, 2019 15:43
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