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

Proposal: Use underscore for internal props #2580

Closed
alitaheri opened this issue Dec 18, 2015 · 9 comments
Closed

Proposal: Use underscore for internal props #2580

alitaheri opened this issue Dec 18, 2015 · 9 comments

Comments

@alitaheri
Copy link
Member

Since we are making the components composable, we will need some way of passing around props that are implementation details. besides the HOCs for themes will also pass down props, these props shouldn't be directly set, put rather via another internal component. Also we can filter names that start with underscore from the documentation. I think this will express the purpose of these props very well. something like _muiTheme instead of muiTheme that can be excluded from the documentation. or many other component that have someway of communicating with their tightly coupled sister components like Toolbars.

To wrap up starting the internal prop names with underscore has the following advantages:

  1. Exclusion from documentations
  2. Avoid confusion as why some props exist (underscore will tell them it will be passed down from other components.
  3. No doc = No commitment! They can be easily changed without breaking user code in any iteration since they will be classified as implementation detail, not parts of the public API.

@oliviertassinari @newoga @chrismcv @mbrookes Please give me your opinion on this.

@oliviertassinari
Copy link
Member

To raise the discussion, another way to do it, would be to blacklist muiTheme from the documentation generation.

@alitaheri
Copy link
Member Author

@oliviertassinari muiTheme might not be the only prop. But it's not a bad idea either :D With internal props we can have much more control over how components communicate with each other. And we can just edit thing as we like since they are private. no more deprecation warning, no more deferred removal and no more breaking change for these private props. We can have them as we like. it's just that underscore is the convention to express it.

@newoga
Copy link
Contributor

newoga commented Dec 18, 2015

I'm agree that we could use some kind of approach for managing/differentiating/hiding internal props from others. I think hiding them from the docs is a good start, and I'm open to naming conventions as well. Like @subjectix said, if it's a truly and internal use only prop, we could technically change the name or naming convention of those props without it impacting users (or having to provide deprecation warnings).

Another thought is that we might be able to use something like ES6 symbols to provide a warning or error if someone attempts to supply a value for an internal prop.

I know in an other issue/pull request we mentioned there were something we'd like to do to improve the docs/codegen. Could it make sense to create an umbrella issue to cover those improvements and include hiding internal props as one of those?

@alitaheri
Copy link
Member Author

@newoga Yeah definitely, an umbrella issue to address these would be great. 👍 Any chance you got the time to compose that? I've got my hands full with the themeManager 😁

@newoga
Copy link
Contributor

newoga commented Dec 18, 2015

@subjectix No problem, I created it the issue here: #2586

@alitaheri
Copy link
Member Author

@newoga thanks a lot 👍

@alitaheri
Copy link
Member Author

Not now with @ignore 😁

@oliviertassinari
Copy link
Member

@alitaheri Thanks for solving this. @ignore is an excellent idea and already implemented 👍.

@alitaheri
Copy link
Member Author

Thanks 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants