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

Comments

Projects
None yet
3 participants
@slimsag
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 (#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 (#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

This comment has been minimized.

Show comment
Hide comment
@pdf

pdf Dec 4, 2017

Contributor

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.

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

This comment has been minimized.

Show comment
Hide comment
@slimsag

slimsag Dec 4, 2017

Member

Fair enough, I agree with that @pdf

Member

slimsag commented Dec 4, 2017

Fair enough, I agree with that @pdf

@slimsag

This comment has been minimized.

Show comment
Hide comment
@slimsag

slimsag Dec 4, 2017

Member

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.

Member

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

This comment has been minimized.

Show comment
Hide comment
@slimsag

slimsag Dec 4, 2017

Member

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

Member

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

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Dec 4, 2017

Member

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.

Member

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