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

Why does the new Context require an explicit interface? #12236

Closed
thenewvu opened this issue Feb 16, 2018 · 8 comments
Closed

Why does the new Context require an explicit interface? #12236

thenewvu opened this issue Feb 16, 2018 · 8 comments

Comments

@thenewvu
Copy link

I just read a post about the new Context, from what I understood, we're creating more problems rather than solving one! Please correct me if I was wrong.

The previous Context, in my opinion, it achieved what called implicit interface, it means you don't need to import explicitly an API to use it, instead you just define what an API should provide (using prop-types) and expect implicitly that it will be provided at runtime. This design helps to decouple API providers from API consumers in the React component tree, which I believe, the original idea of the Context.

implicit interface was also applied in Golang, you would understand what I'm talking about if you have used its interfaces.

But the new Context ... consider below example:

// x.js
const X = React.createContext('x')
...
export default X

// consumer.js
import X from '../../complex/project/x' // the problem!

class Something extends React.Component {
  render() {
    return (
        <X.Consumer>...</X.Consumer>
    )
  }
}

So from now on, we have to import the X context in every consumers of X?

I've worked in this industry for 8 years and got pain enough to understand how bad when we have a complex and heavy dependency graph in our code.

I'm not good at explaining things in English, please help me to explain further the problem we're having here if you got me.

@TrySound
Copy link
Contributor

TrySound commented Feb 16, 2018

It was discussed a lot of times. Take a look at rfc and original thread

@gaearon
Copy link
Collaborator

gaearon commented Feb 16, 2018

You import child components. How is context different?

@thenewvu
Copy link
Author

thenewvu commented Feb 16, 2018

@TrySound Yeah, I read those topics, but I didn't see the problem was listed in the Drawbacks section.
@gaearon Importing child components, you have a tree dependency graph, it will be more flat if you know the right way. Importing the new Context way, you have a spaghetti dependency graph. The difference.

@gaearon
Copy link
Collaborator

gaearon commented Feb 16, 2018

I don't understand what you mean by "spaghetti dependency graph".

Each of your components imports React. Does that make your graph spaghetti? No, it just shows your components are using this library.

Similarly, the components that rely on specific context need to import it to show they depend on it. It's better for large projects because now you don't have the risk of context collisions (because of naming).

You don't have to colocate your context exports with your components. It can be anywhere in the tree. Even in an npm package.

@thenewvu
Copy link
Author

thenewvu commented Feb 16, 2018

"spaghetti dependency graph", I mean, imagine that, you have 10 contexts, 50 consumers in the component tree import those contexts, how does the graph look like? Any form that you can name it?

In the import React case, it's a star graph, it's easier to manage, to delete, to replace, ..., isn't it?

I don't mean how should we organize our graphs. I mean, let's focus on the original problem, a way to provide a piece of data to all components in the tree without messing them up, did we achieve it with the new one?

I understand the new Context solve many problems we're having with the previous one. The new Context was implemented, was merged and seems to be ready to release. I just suggest that we, again please consider the trade off here, is problems it would solve / problems it would issue > 1 ?

@jquense
Copy link
Contributor

jquense commented Feb 16, 2018

problems it would solve / problems it would issue > 1

I can't see how importing Context would cause any problems...the sort of abstract "It makes the dep graph ugly" I don't think is a good reason (I also don't think that's true even)

@thenewvu
Copy link
Author

thenewvu commented Feb 16, 2018

Okay. It seems that only me have the problem we're talking about. If so, I think the issue should be closed now.

Btw, I agree with you on this, @gaearon :

You don't have to colocate your context exports with your components. It can be anywhere in the
tree. Even in an npm package.

But, components have to colocate imports with contexts if we move those components, it's not a problem if we won't move them a lot. NPM packages solve the problem, yes, but they're good for long-term-support code, with in-development code, updating it will take a lot of time, but it's another problem, I won't talk about it.

@gaearon
Copy link
Collaborator

gaearon commented Feb 16, 2018

is problems it would solve / problems it would issue > 1 ?

Judging by this issue being one of the most commented and voted issue in the React repo, I'm confident that the answer is yes.

NPM packages solve the problem, yes, but they're good for long-term-support code, with in-development code, updating it will take a lot of time, but it's another problem, I won't talk about it.

I think what you're describing is not a React API problem, but a dependency management problem. There are existing solutions to this, like Yarn Workspaces, that let you treat local code folders as npm packages, and have imports like my-contexts/theme instead of ../../../../../contexts/theme.

@gaearon gaearon closed this as completed Feb 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants