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

Consider changing (*HTML).Node return type from *js.Object to interface{} #179

Open
slimsag opened this issue Dec 4, 2017 · 5 comments
Open

Comments

@slimsag
Copy link
Member

slimsag commented Dec 4, 2017

Today, the (*HTML).Node method returns *js.Object. For right now, this works well. But in the future, it could be bad for several reasons:

  1. If server-side-rendering (Server-side rendering #23) happens eventually, we may want this to return some other data type (not sure what).
  2. When WebAssemby is supported in Go, it probably won't expose JS through the existing github.com/gopherjs/gopherjs/js package. Or, at the very least, it probably won't be at that exact location (but rather in the stdlib somewhere). The return type will be different.
  3. It's possible that once we figure out testing (determine how best to perform testing #29), we might want it to return some mocked DOM object type.

Now, one could argue that instead of overloading Node() to return an interface{} and making the caller deal with the type, we could instead use more methods (ServerNode(), WebAssemblyNode(), TestNode(), etc.) but I think this would lead to us pulling in too many unwanted dependencies for specific use cases of Vecty, to the point where changing the return type to interface{} is probably warranted.

If nobody objects, we can change the return type to interface{} and add an entry in docs/CHANGELOG.md to reflect it.

@pdf
Copy link
Contributor

pdf commented Dec 4, 2017

My preference would be to leave it as is for the time being - if we have a concrete reason to change it, we'll have more understanding of why. Making people cast stuff without a good reason is not doing them any favours.

@slimsag
Copy link
Member Author

slimsag commented Dec 4, 2017

Fair enough, I agree with that @pdf

@slimsag
Copy link
Member Author

slimsag commented Dec 4, 2017

Something to figure out before 1.0, but after Go gets wasm support and after we figure out what to do about server-side-rendering.

@slimsag
Copy link
Member Author

slimsag commented Dec 4, 2017

https://github.com/gopherjs/vecty/tree/sg/node-interface for if we decide to do this eventually. (wrote it before your comment)

@dmitshur
Copy link
Contributor

dmitshur commented Dec 4, 2017

This sounds like a change that you should defer doing until the very moment you actually need this. Doing it sooner has no benefits IMO, but has downsides in that you might end up changing your mind or gain additional insight by the time you need it.

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