| ▲ | drysart 4 days ago |
| There are inherent limitations with the "execute it once and see what happens" approach; namely that any conditional logic that might be in the mapping function is going to silently get ignored. For example, `db.people.map(p => p.IsPerson ? (p.FirstName + ' ' + p.LastName) : p.EntityName)` would either be seen as reading `(IsPerson, FirstName, LastName)` or `(p.IsPerson, p.EntityName)` depending on the specific behavior of the placeholder value ... and neither of those sets is fully correct. I wonder why they don't just do `.toString()` on the mapping function and then parse the resulting Javascript into an AST and figure out property accesses from that. At the very least, that'd allow the code to properly throw an error in the event the callback contains any forbidden or unsupported constructs. |
|
| ▲ | kentonv 4 days ago | parent | next [-] |
| The placeholder value is an RpcPromise. Which means that all its properties are also RpcPromises. So `p.IsPerson` is an RpcPromise. I guess that's truthy, so the expression will always evaluate to `(p.FirstName + ' ' + p.LastName)`. But that's going to evaluate to '[object Object] [object Object]'. So your mapper function will end up not doing anything with the input at all, and you'll get back an array full of '[object Object] [object Object]'. Unfortunately, "every object is truthy" and "every object can be coerced to a string even if it doesn't have a meaningful stringifier" are just how JavaScript works and there's not much we can do about it. If not for these deficiencies in JS itself, then your code would be flagged by the TypeScript compiler as having multiple type errors. |
| |
| ▲ | drysart 4 days ago | parent | next [-] | | Yeah I'll definitely chalk this up to my not having more than a very very passing idea of the API surface of your library based on a quick read over just the blog post. On a little less trivial skim over it looks like the intention here isn't to map property-level subsets returned data (e.g., only getting the `FirstName` and `LastName` properties of a larger object); as much as it is to do joins and it's not data entities being provided to the mapping function but RpcPromises so individual property values aren't even available anyway. So I guess I might argue that map() isn't a good name for the function because it immediately made me think it's for doing a mapping transformation and not for basically just specifying a join (since you can't really transform the data) since that's what map() can do everywhere else in Javascript. But for all I know that's more clear when you're actually using the library, so take what I think with a heaping grain of salt. ;) | |
| ▲ | Aeolun 4 days ago | parent | prev | next [-] | | Couldn’t you make this safer by passing the map something that’s not a plain JS function? I confess to that being the only thing that had me questioning the logic. If I can express everything, then everything should work. If it’s not going to work, I don’t want to be able to express it. | | |
| ▲ | kentonv 4 days ago | parent [-] | | I think any other syntax would likely be cumbersome. What we actually want to express here is function-shaped: you have a parameter, and then you want to substitute it into one or more RPC calls, and then compute a result. If you're going to represent that with a bunch of data structures, you end up with a DSL-in-JSON type of thing and it's going to be unwieldy. | | |
| ▲ | pcthrowaway 4 days ago | parent [-] | | I suspect there is prior work to draw from that could make this feasible for you... Have a look at how something like MongoDB handles conditional logic for example. | | |
| ▲ | kentonv 3 days ago | parent [-] | | Yeah that's what I mean by DSL-in-JSON. I think it's pretty clunky. It's also (at least in Mongo's formulation, at least when I last used it ~10 years ago) very vulnerable to query injection. |
|
|
| |
| ▲ | skybrian 4 days ago | parent | prev [-] | | Another way to screw this up would be to have an index counter and do something different based on the index. I think the answer is "don't do that." | | |
| ▲ | kentonv 4 days ago | parent [-] | | Hmm, but I should make it so the map callback can take the index as the second parameter probably. Of course, it would actually be a promise for the index, so you couldn't compute on it, but there might be other uses... |
|
|
|
| ▲ | kentonv 4 days ago | parent | prev [-] |
| > I wonder why they don't just do `.toString()` on the mapping function and then parse the resulting Javascript into an AST and figure out property accesses from that. That sounds incredibly complicated, and not something we could do in a <10kB library! |
| |
| ▲ | actionfromafar 4 days ago | parent | next [-] | | Maybe Fabrice Bellard could spare an afternoon. | |
| ▲ | sonthonax 4 days ago | parent | prev [-] | | To the contrary, a simple expression language is one of those things that can easily be done in that size. | | |
| ▲ | kentonv 3 days ago | parent [-] | | But the suggestion wasn't to design a simple expression language. The suggestion was to parse _JavaScript_. (That's what `.toString()` on a function does... gives you back the JavaScript.) |
|
|