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

[List] Use stable context API #13498

Merged
merged 2 commits into from Nov 4, 2018

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Nov 3, 2018

Part of #13394

Not a big fan of MergeWithListContext but ListItem would've been pretty bloated since it has two return statements.

@eps1lon eps1lon added the component: list This is the name of the generic UI component, not the React module! label Nov 3, 2018
@oliviertassinari oliviertassinari added the new feature New feature or request label Nov 4, 2018
@oliviertassinari
Copy link
Member

@eps1lon This looks great. I'm fixing the tests, at least, trying.

@oliviertassinari oliviertassinari force-pushed the feat/core/List/stable-context branch 4 times, most recently from c82264c to 6cac0dc Compare November 4, 2018 22:56
@oliviertassinari oliviertassinari merged commit 371e505 into mui:master Nov 4, 2018
@oliviertassinari
Copy link
Member

@eps1lon If you see anything that is wrong, feel free to raise it :).

@eps1lon
Copy link
Member Author

eps1lon commented Nov 4, 2018

Thanks for cleaning that up! Didn't really have the time to follow through.

I had a different approach though. Will open another PR to get your feedback. I think explicitly looking at a particular child of the component is not really the intention of some of those tests. As soon as we introduce another component into the tree it will likely break again.

@oliviertassinari
Copy link
Member

@eps1lon I ran out of ideas, if you think it can be improved, I'm all in! Also, It's likely that we will use the hook in the future to implement the feature. We need to be resilient to that change. I think that using mount over shallow is already a good first step.

@Ak-lee
Copy link

Ak-lee commented Nov 29, 2018

I find this error only throw out in particular case。
https://github.com/Ak-lee/webpack-server-render-debug
when I enter the directory , only when I run npm run dev:client , then in a new cmd window run npm run dev:server
open the browser at http://'localhost:3333, then the error throw out.
I run the npm run dev:client and npm run dev:server in order to achieve the server automatic从rendering while development.
if I run the npm run dev:client and open the browser at http://localhost:8888 ,there is no error throw out .
if I run the npm run build then npm start and open the browser at http://localhost:3333, both there is no error throw out.
I am sorry that I have't simplify the demo, because the error really throw out in some particular case.

@eps1lon
Copy link
Member Author

eps1lon commented Nov 29, 2018

@Ak-lee please open a separate. I don't understand how this is related to this PR. Maybe #13716 is related?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: list This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants