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 stable context API #13477

Merged
merged 3 commits into from
Nov 1, 2018

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Oct 31, 2018

First step. Removes some noise when tracking down #13394

@eps1lon eps1lon added the docs Improvements or additions to the documentation label Oct 31, 2018
sourceCodeRootUrl: PropTypes.string.isRequired,
};

export default EditPage;
Copy link
Member Author

Choose a reason for hiding this comment

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

"Fun" fact: If you don't export anything eslint will not warn you, the runtime will not throw because read of undefined. React will just throw with "expected component but got object".

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 31, 2018

Well done. I like you have extracted blocks of logic into render props, waiting for hooks to be available.

@@ -0,0 +1,5 @@
import React from 'react';

const PageContext = React.createContext();
Copy link
Member Author

Choose a reason for hiding this comment

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

That's missing the previous default values. They should be present for documentation purposes.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe a comment in this case?

Copy link
Member Author

Choose a reason for hiding this comment

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

Documentation gets outdated. With actual values vscode is able to acquire type information and enables autocomplete.

Copy link
Member

Choose a reason for hiding this comment

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

Documentation gets outdated.

Do does default values that are never used.

With actual values vscode is able to acquire type information and enables autocomplete.

Ok, that's the why. I'm adding them back.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for raising.

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe that this is actual the typescript language server so any editor (plugin) that is leveraging that benefits. I just like my autocomplete ;)

@oliviertassinari oliviertassinari merged commit ac1e742 into mui:master Nov 1, 2018
@eps1lon eps1lon deleted the docs/stable-context-api branch November 1, 2018 07:09
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.

2 participants