New package: mdg:validated-method - Meteor methods with better scoping, argument checking, and good defaults

[jasoncchild]

To be clear, are you saying that from hence-forth I must use simple-schema to build Meteor apps? Or are you saying this is just considered best-practices by MDG?

[deanius]

Thoughtful, and I like the spirit of it generally! Still, Promises are part of core now, and they’re not used, that’s my only critique. Can you omit the callback parameter and get a Promise ? If not, is there any guarantee of whether the callback is invoked sync or async, or multiple times, for the simulation, and the eventual real ?

[sashko]

Ah I was hoping you would comment! What do you think is the best way to integrate promise support here? I feel like it’s not yet a good time to switch to always returning a promise, and doing it only sometimes seems weird. Some options:

1. Add support for mixins, have a promise mixin that wraps call
2. Make a subclass class PromiseMethod extends Method that wraps call
3. Add a new method callAsync or callPromise

As for the callback, it has the same semantics as Meteor.call - is there some case where that invokes the callback twice, or synchronously? This wrapper could be a good place to fix that, if so.

[sashko]

You don’t need to use simple-schema, but that will be the path we recommend. The idea is that the core Meteor framework can remain flexible (and actually become more flexible) and the guide project can be completely opinionated.

[tmeasday]

Add a new method callAsync or callPromise

I’m not sure if you saw this https://github.com/meteor/meteor/pull/5005 but my impression was that making methods return promises is a non-trivial exercise for some complicated reasons…

[waldgeist]

You’re making a good point by mentioning Astronomy here. I think that Meteor is definitely lacking a robust and standardized way to set up model classes and hook them to the underlying collections (including validation) in a DRY way. Something that BaseModel and Astronomy are trying to achieve.

I’m using neither of them at the moment (because I did not have enough time to rework my code), so I know the pain that arises from not having such a model wrapper. If I change a field in one of my model classes, I also have to update my SimpleSchema for the collection (which I am not forced to use, but helped me a lot to detect issues), and this of course is double work.

So, instead of just hooking SS into this new way of method invocation, wouldn’t it be better to introduce a more object-oriented modeling paradigm to Meteor, by which the method invocation itself could then decide which validation to use, based on the types of the input parameters? I am speaking of a way of introducing stronger typing to Meteor.

To keep things simple for beginners, this strong typing mechanism could be made opt-in (just like autopublish), but for a maintainable app it would be a must.

[serkandurusoy]

We did actually spend some time looking around and calling for suggestions: https://github.com/meteor/guide/issues/534

Looking around (with only one more person other than you and aldeed) is one thing, bringing this up with the community, like you guys did on the Blaze topic is another thing. Besides, it was a “guide” issue, not a “core” issue that revolved around redefining how methods work.

JSON Schema seems to be the closest thing to a “standard” right now, but I think it’s a long way away from being that

So yeah, instead of embracing and contributing to the standards, let’s do something else (Blaze?) and then when the broader JS community finally comes up with a standard, let’s ditch all the hard work and the effort?

f we’d been quicker to embrace SS, then I would guess Astronomy … would probably have used SS for its schemas, and we’d be in a better place right now

Well, it’s not what happened and you really can’t say that on behalf of Astronomy’s author. In fact aldeed himself said the exact opposite, that he would have not bothered creating SS had astronomy existed back then. But that’s besides the point. I’m not advocating astronomy here.

I don’t think we are “shutting out alternatives”, except if you mean choosing something to recommend in the guide and elsewhere

Again, are we talking about the guide or a package that’s very likely to become core? For the latter, taking in SS is not recommending anything, it is a bold statement for SS and against anything else. Subclassing etc are then just technical politics and far cries of the remaining alternatives.

I think the consensus is that it’s a very good idea for us to do this.

Again, whose consensus? From the github issue you’ve linked, it looks like two or three people.

So my overall point is, I think this decision of yours is a statement of how MDG operates and how Meteor and the community is governed. And it just does not help heal the many similar catastrophies we’ve had until now.

[msavin]

Would this be satisfied by explaining all of the stuff and then suggesting people use this package (or one like it) or do you mean that we should just teach people to use the boilerplate every time?

I’d say every time since its important to know how it works and what problems it solves. It can be like a pre-requisite. Maybe the package could be offered in the end as a shortcut “now you know, let’s save some time” - but I’m not comfortable with this being a core package.

[jhartma]

This is a big improvement of methods, and I would happily switch over to use the package. I understand @serkandurusoy though. Playing with the package is one thing, but using it in business apps is another with real money at stake.
It would help to know if it will become part of core or being maintained in the long run, so @sashko or @tmeasday maybe can tell something on their plans or long term commitment?

[shilman]

@aldeed after all this blaze drama, news of simpleschema v2 node package is a surprising ray of sunshine!!!

[shcherbin]

@aldeed after all this blaze drama, news of simpleschema v2 node package is a surprising ray of sunshine!!!

Yeah, now you will have to rewrite your schemas too!

[copleykj]

I think we should add a checkPattern option that can be used instead of schema. However, I think in the long term standardizing around a more feature-complete schema definition language like simple-schema would be better; like I said above, check has a lot of problems and there’s no point in having two separate things if simple-schema has more features and a lot of people are already using it.

So if check goes by the wayside in favor of Simple Schema, should be then possibly develop a pattern for using it in our publications as well?

[serkandurusoy]

An interim would be to use check(input, schema) since SS already supports use as a match pattern.

[copleykj]

Right, I’m just wondering what happens when check is no longer maintained.

[serkandurusoy]

Check is a fairly straightforward package which has not seen too many major updates since its inception. I guess one could continue using it with some piece of mind. That being said, I’ll no longer be surprized if/when MDG decides pulling things from under me.

[jasoncchild]

Ah, thanks! I was confusing the platform with the Guide Thanks again for pushing out a best practices document for us all!

[thomasyajl36]

Really like your point here of Meteor being flexible but the Guide being opinionated. As noob to development, I always feel like I’m doing something wrong and missing out on important features. It can be discouraging sometimes, so that’s where I think the Guide comes to the rescue. My humble suggestion for the Guide (and Meteor docs perhaps):

I’m a huge fan o the way Stripe structured their documentation: https://stripe.com/docs/api#intro , with the 3 panels split screen showing the menu | explanations | code syntax + examples. It’s really easy to navigate and grasp what’s going on. The examples are few but encompassing, giving you full acknowledgment of the features available. The overall formatting is outstanding.

[sashko]

To start out, I want to say that if building/releasing this kind of package is not the best way forward, then we won’t do it. This is definitely a request for feedback rather than a set in stone thing.

Besides, it was a “guide” issue, not a “core” issue that revolved around redefining how methods work.

So here’s how this package came about:

1. We needed to write an article for the guide about forms and methods.
2. We started thinking about the best ways to call methods, in line with the patterns @tmeasday had been using at Percolate.
3. In the initial exploration of the article, the pattern for defining and calling methods became quite verbose - that is from an earlier exploration of best practices for methods.
4. Even that verbose pattern didn’t handle returnStubValue, and didn’t let you actually display helpful errors on the client when a method invocation failed to validate.

We figured that recommending 15+ lines of boilerplate for every single method call was a bit much, but anything less was not using the Method system to its full potential. Our analysis was that any serious project would need to build its own wrapper class for methods.

Analysis of this thread

Please let me know if I’ve mis-represented your position.

For:

1. @efrancis says “it’s really great”
2. @avital says “I love the idea of eliminating magic strings, but I don’t want to be forced to use simple-schema”
3. @waldgeist says “Good idea to make method invocation easier, this has some built in features that are very hard to find otherwise, but we shouldn’t tie it to simple-schema”
4. @Steve says “meteor-call-sync has been doing this for a while, having returnStubValue and throwStubExceptions on by default is a good idea.”
5. @jhartma says “This is a big improvement of methods, and I would happily switch over to use the package. However, I wouldn’t want to use it in a real business app without a guarantee of support.”

Against:

1. @msavin says “The guide should explain the underlying issues and teach people how to make a method wrapper for themselves. We should make sure this package is not presented as a replacement for methods.”
2. @serkandurusoy says "I really don’t want MDG to start building things around simple-schema, since it’s not a standard and there are some alternatives that might be better. The proposed API seems much better than the previous method API, but I’m uncomfortable for new packages to be introduced in this way.

Neither:

1. @SkinnyGeek1010 says “I also wrote a wrapper for methods, which looks different and enforces some modularity in a different way. I think the most important part is understanding what methods exist, and making them easily testable.”
2. @deanius says “A good method API would include support for promises.”

Conclusions

It’s clear from the conversation that the current method API isn’t ideal, and there is some boilerplate involved to write good, maintainable methods. A wrapper package isn’t necessarily the best way to deal with the situation, but if we had to reproduce the boilerplate in every place in the guide where methods are involved, it would be very difficult to read.

Decouple simple-schema: The built-in simple-schema integration set off some alarms for people. aldeed proposed a simpler API that allows any type of schema to be used, as long as you can write a function that throws validation errors. I think going with that is a good idea. While we are definitely going to be writing code samples for the Meteor guide using simple-schema for a lot of stuff, there’s no reason for it to be hardcoded into anything if it doesn’t have to be.

Guarantee of support: If we release this as a package that we intend people to use, it needs to be maintained. Methods are a crucial part of the Meteor app writing experience, and you wouldn’t want to have bugs in that system introduced by some wrapper code.

Another option - DIY method wrappers?

There’s one other option other than introducing a package - teach people to write their own method wrapper that encodes some of the same practices. The main downside here is that encouraging people to write custom wrappers for methods will significantly cut down how easy it is for someone to understand a Meteor app they haven’t worked with before. First you’ll have to read and understand that particular project’s method wrapper code. So there are benefits to a standardized wrapper package.

[shcherbin]

Wow, I like this analysis. Would love to see such things in another topics.

Great job, @sashko!

[serkandurusoy]

@sashko although the for/against/neither breakdown is somewhat convoluted in its own regard, I think this is a great breakdown and it fills me with joy to see how well you/mdg can assess a given situtation with all of its complexity. Kudos.

I sincerely believe that all arguments pro/con given for the blaze announcement applies here as well, in fact to everything Meteor and I guess the overall concerns are about the API stability and long term support. For this thread specifically, I wish the proposed method api did take cue from the openness of the view layer such that things like schemas/models could be planned out to be more interchangeable.

Apart from that, I find the direction quite pleasing and in line with the overall feel of Meteor.