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 not wrap the API with hand writed js interop ? #246

Open
Kleak opened this issue Mar 7, 2016 · 5 comments
Open

Why not wrap the API with hand writed js interop ? #246

Kleak opened this issue Mar 7, 2016 · 5 comments

Comments

@Kleak
Copy link
Member

Kleak commented Mar 7, 2016

I thinks everythings is in the title.
And because if there is a problem is more difficult to fix the bug..
And like that we can done a better wrapper than just wrap things
take an example for the tcpSocket and Socket API everybody re-wrap this part because it's not really the Dart way of doing things..

@Kleak Kleak changed the title Why not use the js interop for wrapping chrome app/ext API ? Why not wrap the API with and writed js interop ? Mar 7, 2016
@Kleak Kleak changed the title Why not wrap the API with and writed js interop ? Why not wrap the API with hand writed js interop ? Mar 7, 2016
@adambender
Copy link
Contributor

Well at the time this was created jsInterop didnt give you a lot of type safety and was also very verbose. Using chrome.dart allowed you to at least use code that worked more like a dart library than a collection of associative arrays. In light of the recent updated to jsInterop, it might be worth re-looking at the use case. There are still some things that are lost like callback <-> stream conversion that this this library mostly handles.

Of course keep in mind that under the hood, chrome.dart just uses interop so it acts as a drop-in replacement for all the boilerplate you would have to write yourself to do the same thing.

Regardless, the point is well taken but at the moment there just isnt that much effort required to keep it alive. Are you suggesting, instead, that we just toss out the repo? or mark it deprecated?

@Kleak
Copy link
Member Author

Kleak commented Mar 7, 2016

Yeah sure it already use jsinterop but there are some problem into the library and i think the idl parse is a big step for people to help improve the library than a hand writed version.

We cann't just toss out the repo because some people use this package neither mark as deprecated for the same reason before we have something that will replace it.

But i think we need a more stable library for writing chrome app/ext. Where everybody can easily read and fix things.

Ps: i'm actually wrapping nodejs api, in order to wrap electron for dart and it's pretty quick. Maybe i/we can do the same for chrome app/ext api and after that thinks about add a layer that dartify the API ?

@Kleak
Copy link
Member Author

Kleak commented Mar 8, 2016

The most difficult part is the first version after that the API doesn't change a lot.

@adambender
Copy link
Contributor

Okay so I think I misunderstood your original query. I thought you were questioning why the project exists not the use of the IDL Parser. This debate has come up before on this project and it was determined then that the IDL Parser is actually the best solution for a couple reasons:

  1. Rolling new APIs is at this point, a single click operation + fixing small issues with new api types. As such we arent struggling to keep up with new releases and as a consequence don't feel the need to grow the contributor base in order to do so. There maybe other reasons to grow but speed of releases isn't one at this point.

  2. The automated process never misses an API and deterministically produces correct code for each API. This ensures we always catch new things and remove those unsupported APIs.

These two reasons make a pretty compelling case, in my mind, for sticking with the parser. As it is today two people can effectively maintain the entire project. Switching to a handwritten api would definitely allow more people to contribute, but I believe it would also require more people to contribute keep up with all the changes. That makes me nervous because like most open source maintainers keeping this library alive isn't my full time gig so the handwritten API approach seems like it increases the complexity of managing pull requests and keeping things up to date. If you have concerns on the other side that outweigh Im happy to hear them out.

Id be curious where you have found instability, all of the code is autogenerated based on public chrome apis, so if there is an issue with bugs or things moving around then it is likely down to chrome.

Like I mentioned, we have discussed the idea of going with a handwritten API before, perhaps even bootstrapping it by running the IDL parser one more time and then iterating by hand from there. From where we are sitting though, the process we have is working well enough. Of course it probably isnt perfect, but whatever is? Do you have some specific things youd like to see changed that justify the move to a handwritten API?

Im not convinced that 'everyone should be able to read and fix things' because I think that sets an unreasonably low bar for this kind of project. Im open to being convinced otherwise though, just make the case.

@Kleak
Copy link
Member Author

Kleak commented Mar 9, 2016

You have right on the fact that rolling new API is very easy with this way of doing things.
But there is some bug in the issues tracker that are not cover like missing property in #201 or in #183.
And after that there is weird bug like #245
And maybe other that are not found...
So yes it's a one click release but with some disadvantage.
I don't say hand written code will be better since there is a huge work for cover all the api but after i think the work will be much small.
The new jsInterop is pretty easy to use and surely way far easy than investigate in the idl parse if people want to participate.
There are thread off on both side.

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

2 participants