ceifa
Add MultiReturn support
It kinda with breaking changes (in filesystem tests) but i tryed my best to bring multret support as softly as i can.
code that failed wasn't changed. so it looks like skip this one for me. What do you think, @ceifa?
code that failed wasn't changed. so it looks like skip this one for me. What do you think, @ceifa?
can you run npm run lint? I think it will fix the build
Im not confortable about this change. Its a little bit weird that the return type changes if you are returning a single value or a multi value, if we understand that we should support it, we should stick with always returning the array, but this causes other problems, like we cant write a function in lua and just translate it to JS, since one will return a value, and the other an array, example:
function sum(x, y) -- return an array in JS
return x + y
end
function sum(x, y) { // return a number in lua
return x + y
}
@timstableford do you have any thoughts?
@ceifa there is soft support of mulret. so basically your first code will return one and only one return. and only if you wrap add , x it will return mulret. So anyone can check return type (which is fine) and do what they need to.
@ceifa i was thinking about this issue too. but mine impl wont bring any breaking changes to all exsisting projects. So if we make it always return an array it will cause many breaking changes.
For now we can use kinda generics
const [a,b,c] = lua.doString<MulRet>(`code`); // return all returns
const a = lua.doString<any>(`code`); // return first one return
Im not confortable about this change. Its a little bit weird that the return type changes if you are returning a single value or a multi value, if we understand that we should support it, we should stick with always returning the array, but this causes other problems, like we cant write a function in lua and just translate it to JS, since one will return a value, and the other an array, example:
function sum(x, y) -- return an array in JS return x + y endfunction sum(x, y) { // return a number in lua return x + y }@timstableford do you have any thoughts?
That's a tough one. I think there's two cases though that could be reasonably kept consistent. When calling into a Lua program from JS as an entrypoint like with doString I'd personally have it always return an array. Then for functions that are passed back to JS from Lua have it only use the single return value, like when calling JSON.stringify from Lua and passing a replacer function.
Perhaps I'm just struggling to see a use case for that half of it though? @ParadiseFallen out of curiosity is this a just for the fun of it PR or did you have a specific use in mind?
there is no any special use cases but it would be nice to have that feture. if lua uses mulret we can support it softly.
For now there is some built in functions that i want to call and can't bc there is no mulret.
@timstableford why we won't use generics? its seems exatly what we want. so by default it will be just single return and if you pass expected return as <MulRet>() it will wrap it in mullret. even if there is only one return
I'm not against using generics but it'd be necessary to link it to something runtime. As far as I understand TypeScript that could be an argument to the function to say which return mode to use or an alternate function. I'm having a bit of trouble imagining it at the mo but tl;dr I'm sure you can go that route if you work through quirks
@timstableford well. if you want use it in js then we can made kinda
function doString(code: string, returnStategy: IReturnStrategy = SingleReturn)
{
// just as normal code
return returnStrategy.makeReturn(thread);
}
but need to look for how to hookup it when you want mulret (bc we need start top)
or even just use enum for that
export enum ReturnType{
SingleValue = 0,
MultipleValues = 1,
}
That's a tough one. I think there's two cases though that could be reasonably kept consistent. When calling into a Lua program from JS as an entrypoint like with doString I'd personally have it always return an array. Then for functions that are passed back to JS from Lua have it only use the single return value, like when calling JSON.stringify from Lua and passing a replacer function.
You are right, it would break implementations like that, which is very common.
@ceifa there is soft support of mulret. so basically your first code will return one and only one return. and only if you wrap add , x it will return mulret. So anyone can check return type (which is fine) and do what they need to.
I'm not a fan of this approach, returning always the same type would be better for maintainability.
I will let the PR open for now, and think better about it later
@ceifa any updates? i suggest move to always returning an array expecting multiparams
@ParadiseFallen I've come up with something that might make this more palatable (to me, I can't speak for @ceifa who will be merging it).
What you could do is overload the function signatures like this https://stackoverflow.com/a/13212871 For instance with doFile
public doFile(filename: string, returnMode: 'multi'): Promise<MultiReturn>;
public doFile(filename: string, returnMode?: 'single' | 'multi'): Promise<any> {
// Implementation here
}
That would mean the current function signatures would remain compatible and return a single item but if return mode was set to multi the return type would be MultiReturn instead
Автор
@ParadiseFallen I've come up with something that might make this more palatable (to me, I can't speak for @ceifa who will be merging it).
What you could do is overload the function signatures like this https://stackoverflow.com/a/13212871 For instance with doFile
public doFile(filename: string, returnMode: 'multi'): Promise<MultiReturn>; public doFile(filename: string, returnMode?: 'single' | 'multi'): Promise<any> { // Implementation here }That would mean the current function signatures would remain compatible and return a single item but if return mode was set to multi the return type would be MultiReturn instead
I already proposed that. we dont even need overload for that.
You're right that you proposed a return mode, which I generally liked but the explicit return type of multireturn I think will make it easier.
I've just been reading through the comments again after your prompt and the other part that I think was tripping us up was that functions returned from Lua to JS would need to be single return for JS compatibility. I'd be okay with you passing it in as an option to the engine whether to do single returns or multi-returns in that circumstance.
It's been a while since I've put any thought into this, so sorry for rehashing a bit.
@tims-bsquare why just single return? its easilly supported with just mulret
const mulret = call();
const [r1, r2, r3] = call();
For JS interoperability unless I'm misunderstanding you. Consider exposing the JSON.stringify function to Lua as an example. So Lua does
JSON.stringify({}, function (replacer) return something end)
stringify expects a function that return a single argument, by not keeping single as default the above breaks
@tims-bsquare there many pain points on example like that. also it looke like
-- lua
-- return generic builder
someJsFunc(function (startIndex)
-- some wrap for example.
return function ()
local x = startIndex;
-- indexer
return function ()
x = x + 1;
return x;
end
end
end)
this example will breaks. same for js side.
Sorry @ParadiseFallen I'm not following the problem you're describing. Currently the example I showed works and I'm not sure what your function is showing
@tims-bsquare it's showing another problem, that not mentioned there. sometimes wrapper breaks
@tims-bsquare it's showing another problem, that not mentioned there. sometimes wrapper breaks
I'm a little bit confused too, can you elaborate the problem you are showing?
My two cents as a user:
In Lua, a = (function() return "a", "truncated" end)() does not assign a table to a but simply truncates the stack (i.e. a = "a"). And I think it is only natural to keep this as default.
Alternatively, if multiple return values are wanted, I can always wrap them in a table like return { "a", "b" } and can comfortably achieve anything I want without necessitating this MULTI_RET change.
Some more justifications:
-
The Array when >= 2 (JS) approach:
In Lua, changing from
return 1toreturn 1, "extra"is usually not a breaking change due to how the stack truncation works. However, with the current PR (return 1 => (JS) 1,return 1, 2 => (JS) [1, 2]), it will. So either we stick with our previous truncation approach or we always return an array. -
The Always array approach:
Always returning an array has its own problem: it poses a limit on the library's capability - Lua functions will no longer be able to simulate any JS function that can return anything.
The following matrix outlines whether the user will be able to do something with code change in only one language:
| Get a single value | Return multiple values | |
|---|---|---|
| Previous (JS side) | / | Function wrapper |
| Previous (Lua side) | / | Wrap in tables |
| Always array (JS) | Function wrapper | / |
| Always array (Lua) | You can't | / |
| Array when >= 2 (JS) | Sometimes / wrapper | Sometimes / wrapper |
| Array when >= 2 (Lua) | Sometimes / wrapper | Sometimes / wrapper |
Therefore, speaking as a user, I am against this breaking change. (The overloading approach is non-breaking and feels better.)
lets take this as example
-- IdGenerator
return function (start,step)
local current = start
return function ()
current = current + step
return current
end
end
-- main
local IdGenerator = require 'IdGenerator'
-- this method will called from js
function tryCreateGenerator(start)
if start < 0 then
return false
end
return true, IdGenerator(start, 1)
end
return tryCreateGenerator
Then lets call it from js
const factory = new LuaFactory();
const engine = await factory.createEngine();
const tryCreateGenerator = engine.doString(/*code goes here*/);
// we can also get `tryCreateGenerator` just by engine.global.get('name').
// and now we facing few issues
// first one is mulret. it won't work
// second one is returning of lua function. it just breakes
const [created, generator] = tryCreateGenerator(2) // won't work
also it shows one more issue. if generator() will return 2 values
ie
function createGenerator(start, step)
local current = start
-- generator itself
return function()
current = current + step
if current > 100 then
return false, current - step
end
return true, current
end
end
it won't work. bc internal function will called on js side. and it must return mulret. but we can't specify which value we expect. mulret or not. So this option muse be specified a bit higher that just doString or doFile