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

bikeshed: is contract really needed? #9

Open
millermedeiros opened this issue Apr 16, 2015 · 7 comments
Open

bikeshed: is contract really needed? #9

millermedeiros opened this issue Apr 16, 2015 · 7 comments
Labels

Comments

@millermedeiros
Copy link
Contributor

JavaScript is a dynamic, loosely-typed, language so type checking is really a code smell. Also by shifting the validation logic into the manager you need to update 2 files when the API changes, which doesn't seem like a good idea.

I know for sure that this kind of type checking won't make the Calendar code easier to understand and/or catch real bugs; you still need to read the implementation to understand that 'string' refers to an ID and that 'object' should actually implement a certain interface to be usable...

I would rather see the service automatically throwing an error when trying to call a method that doesn't exist (without the need of a contract). And also list the available events on the service itself (or implement something like signals that ensures you can only dispatch and listen to known event types).

IMO the manager should only know about the src and type. In most cases the manager will probably be defined on the front-end and the services will be on the back-end, sometimes even on separate repositories.

@wilsonpage
Copy link
Contributor

The contract is less about catching errors in your own code and more about a client and service adhering to an agreed API. The goal is for someone (hacker/partner) to be able to replace a view panel and only have to look at the contract, never the service code.

The service should understand that with any code changes must be backward compatible with the contracted API. If this is not possible I think we'd have to reversion the service and still somehow support the old API should it be requested by an old client.

// Kept around to support old client code
service('my-service-1', ...);

// New service for latest client code
service('my-service-2', ...);

These are early ideas and still in debate.

@millermedeiros
Copy link
Contributor Author

millermedeiros commented Apr 17, 2015 via email

@wilsonpage
Copy link
Contributor

I see where you're coming from, but this is something Vivien is insisting on from lessons learnt in Gecko land. The contract should be written by the service-owner and read by the the client-owner. Perhaps it should be installed within the service directly so that the client never has to touch it?

threads.service('my-service')
  .contract(require('./contract'))
  .method('foo', ...);

This most important thing is that these contracts are 'versioned' in some way and the service-owner is aware that changing it will break old clients that depend on it. The ease of adding/removing/changing method names/signatures in code seems far to fragile. A contract of sorts makes this more difficult.

@millermedeiros
Copy link
Contributor Author

I would maybe add the version number to the service name .service('[email protected]') if that is the case (only 2 digits on the version because bug fixes shouldn't affect the API) -- we can forget to update the version number the same way as we can forget to update the contracts; but at least only have to edit a single file...

Maybe on Gecko it made sense but for my limited use cases I can't see how it would improve the workflow. It would be great if Vivien could share his view, maybe I'm overlooking something important.

@asutherland
Copy link

Has any thought been given to standardizing contracts on using (a subset of) the WebIDL grammar (http://heycam.github.io/webidl/) for which a fancy parser is available (https://github.com/darobin/webidl2.js)?

The main advantages would be:

The main disadvantages would be:

  • Potential perceived overkill. The counter-argument to this is that a contract/interface definition language is going to tend to expand or either fail to deal with more complex cases. And one needn't use everything WebIDL allows.
  • Confusion about what WebIDL allows versus what threads actually allows. Documentation should be able to describe this, and whatever consumes the parser output can certainly throw errors and explode for enforcement.

@wilsonpage
Copy link
Contributor

I'm not that keen on introducing WebIDL for the following reasons:

  • We would require some kind of build step to transform the WebIDL contract into something that can be checked in JS land. This adds complexity to library implementation and usage.
  • I think typeof or instanceof checks or deep type checks on object keys should be sufficient, and is fairly self explanatory.
  • We risk alienating users. From my experience only people working with platform specs are WebIDL literate.

@asutherland
Copy link

For parsing/build steps, I was thinking you'd have a dynamic mode and a built/optimized mode. RequireJS/r.js plugin loaders support this idiom, as do webpack/babel/etc. The advantage to something like this is that you could use codegen to unroll the type checks so they can JIT and be type-stable.

For typeof/instanceof being sufficient, this surprises me somewhat because I got the impression that the enforcement of contracts would be part of enabling our extensibility efforts. (Specifically for security reasons. It makes sense to me to roll the enforcement into whatever is doing the threads-type layer and to have it allow the contract to be as tightly specified as desired.) It'd probably be worth checking with @freddyb or other security team people for what their vision is if we're exposing unified services.

Alienating users is a good concern. From what I've been hearing, adoption of threads.js is effectively mandatory for Gaia apps (and Gaia developers should be at least a little WebIDL literate, or at least there are huge upsides for them to become WebIDL literate-ish). I honestly would expect many people to find threads.js and the new iframe architecture heavyweight in general and unlikely to be adopted by anyone who doesn't want to do exactly what Gaia does.

If there's some other standard for defining these types of things that's more idiomatic, that would generally satisfy my concerns. The heart of my concern is creating a de facto new standard, and one that isn't sufficiently expressive to meet the eventual security concerns we'll have.

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

No branches or pull requests

3 participants