| |
| ▲ | btown 4 days ago | parent | next [-] | | > your function needs to have no side effects I'm trying to understand how well this no-side-effects footgun is defended against. https://github.com/cloudflare/capnweb/blob/main/src/map.ts#L... seems to indicate that if the special pre-results "record mode" call of the callback raises an error, the library silently bails out (but keeps anything already recorded, if this was a nested loop). That catches a huge number of things like conditionals on `item.foo` in the map, but (a) it's quite conservative and will fail quite often with things like those conditionals, and (b) if I had `count += 1` in my callback, where count was defined outside the scope, now that's been incremented one extra time, and it didn't raise an error. React Hooks had a similar problem, with a constraint that hooks couldn't be called conditionally. But they solved their DX by having a convention where every hook would start with `use`, so they could then build linters that would enforce their constraint. And if I recall, their rules-of-hooks eslint plugin was available within days of their announcement. The problem with `map` is that there are millions of codebases that already use a method called `map`. I'd really, really love to see Cap'n Web use a different method name - perhaps something like `smartMap` or `quickMap` or `rpcMap` - that is more linter-friendly. A method name that doesn't require the linter to have access to strong typing information, to understand that you're mapping over the special RpcPromise rather than a low-level array. Honestly, it's a really cool engineering solve, with the constraint of not having access to the AST like one has in Python. I do think that with wider adoption, people will find footguns, and I'd like this software to get a reputation for being resilient to those! | | |
| ▲ | da25 4 days ago | parent [-] | | This !
Using the same name, i.e. `.map()` is a footgun, that devs would eventually fumble upon.
`rpcMap()` sounds good.
cc: @kentonv |
| |
| ▲ | tonyg 4 days ago | parent | prev | next [-] | | Is .map specialcased or do user functions accepting callbacks work the same way? Because you could do the Scott-Mogensen thing of #ifTrue:ifFalse: if so, dualizing the control-flow decision making, offering a menu of choices/continuations. | | |
| ▲ | kentonv 4 days ago | parent [-] | | .map() is totally special-cased. For any other function accepting a callback, the function on the server will receive an RPC stub, which, when called, makes an RPC back to the caller, calling the original version of the function. This is usually what you want, and the semantics are entirely normal. But for .map(), this would defeat the purpose, as it'd require an additional network round-trip to call the callback. | | |
| ▲ | qcnguy 4 days ago | parent | next [-] | | What about filter? Seems useful also. | | |
| ▲ | kentonv 4 days ago | parent [-] | | I don't think you could make filter() work with the same approach, because it seems like you'd actually have to do computation on the result. map() works for cases where you don't need to compute anything in the callback, you just want to pipeline the elements into another RPC, which is actually a common case with map(). If you want to filter server-side, you could still accomplish it by having the server explicitly expose a method that takes an array as input, and performs the desired filter. The server would have to know in advance exactly what filter predicates are needed. | | |
| ▲ | svieira 4 days ago | parent | next [-] | | But you might want to compose various methods on the server in order to filter, just like you might want to compose various methods on the server in order to transform. Why is `collection.map(server.lookupByInternalizedId)` a special case that doesn't require `server.lookupCollectionByInternalizedId(collection)`, but `collection.filter(server.isOperationSensibleForATuesday)` is a bridge too far and for that you need `server.areOperationsSensibleForATuesday(collection)`? | | |
| ▲ | kentonv 4 days ago | parent [-] | | I agree that, in the abstract, it's inconsistent. But in the concrete: * Looking up some additional data for each array element is a particularly common thing to want to do. * We can support it nicely without having to create a library of operations baked into the protocol. I really don't want to extend the protocol with a library of operations that you're allowed to perform. It seems like that library would just keep growing and add a lot of bloat and possibly security concerns. (But note that apps can actually do so themselves. See: https://news.ycombinator.com/item?id=45339577 ) |
| |
| ▲ | 5Qn8mNbc2FNCiVV 2 days ago | parent | prev [-] | | Couldn't this be done in some way when validation exists, that the same validation is used to create a "better" placeholder value that may be able to be used with specific conditional functions? (eq(), includes(), etc.) |
|
| |
| ▲ | svieira 4 days ago | parent | prev [-] | | Doesn't this apply for _all_ the combinators on `Array.prototype` though? Why special-case `.map` only? | | |
|
| |
| ▲ | spankalee 4 days ago | parent | prev | next [-] | | But you also can't close over anything right? I did a spiritually similar thing in JS and Dart before where we read the text of the function and re-parsed (or used mirrors in Dart) to ensure that it doesn't access any external values. | | |
| ▲ | kentonv 4 days ago | parent [-] | | You actually CAN close over RPC stubs. The library will capture any RPC calls made during the mapper callback, even on stubs other than the input. Those stubs are then sent along to the server with the replay instructions. |
| |
| ▲ | fizx 4 days ago | parent | prev [-] | | Also, your function needs to be very careful on closures. Date.toLocaleString and many other js functions will be different on client and server, which will also cause silent corruption. | | |
| ▲ | kentonv 3 days ago | parent [-] | | If you invoke `Date.toLocaleString()` in a map callback, it will consistently always run on the client. | | |
| ▲ | fizx 3 days ago | parent [-] | | I don't see how this very contrived example pipelines: client.getAll({userIds}).map((user) => user.updatedAt == new Date().toLocaleString() ? client.photosFor(user.id) : {})
or without the conditional, client.getAll({userIds}).map((user) => client.photos({userId: user.id, since: new Date(user.updatedAt).toLocaleString()})
Like it has to call toLocaleString on the server, no? | | |
| ▲ | kentonv 3 days ago | parent [-] | | Neither of these will type check. You can't perform computation on a promise. The only thing you can do is pipeline on it. `user.updatedAt == date` is trying to compare a promise against a date. It won't type check. `new Date(user.updatedAt)` is passing a promise to the Date constructor. It won't type check. |
|
|
|
|