Default Tool arguments

[TomasPetrauskas]

Describe the bug
By passing an array of default tool arguments to IModelApp.toolAdmin.defaultToolArgs the arguments do not get parsed correctly. In the async run function line 821 of the tool.js file, the array does not get spread with the spread operator and instead becomes an array of an array. You will receive an error that the tool's viewport is not setup correctly as the viewport is at that point set as an array with the arguments.

To Reproduce
Steps to reproduce the behaviour:

1. Add default tool arg array
2. Attempt to run the default tool
3. You will receive an error once trying to use the tool that the tool's viewport does not contain some function.

Expected behavior
The arguments should be parsed separately not enclosed into another array.

Desktop (please complete the applicable information):

• OS: Windows 10
• Browser: Chrome
• Version: 106.0.5249.91
• iTwin.js Version: 3.3.0

Additional context
This should be an easy fix as the variables being passed in the function should not contain the spread operator (...args) as this is what breaks it.
This issue can simply be recreated by running this piece of code:

let defaultToolArgs = [1,2,3];
function hej(a, b, c) { console.log({a,b,c}) }
function test(a, ...b) { hej(...b) }
test("hej", defaultToolArgs)

If this code is run:

let defaultToolArgs = [1,2,3];
function hej(a, b, c) { console.log({a,b,c}) }
function test(a, b) { hej(...b) }
test("hej", defaultToolArgs)

We receive the expected behaviour.

[TomasPetrauskas]

Example code being run:

Debugging the tool.js code:

Attaching some screenshots for more reference.

[bbastings]

I think this would let you do what you want without making public api changes (below)?

public async startDefaultTool(): Promise {
(remove) if (!await IModelApp.tools.run(this.defaultToolId, this.defaultToolArgs))
(add) if (!await IModelApp.tools.run(this.defaultToolId, ...(this.defaultToolArgs || [])))
return this.startPrimitiveTool(undefined);
}

@TomasPetrauskas Agree? Not sure whether there is a prettier way to deal with defaultToolArgs allowing undefined...

[TomasPetrauskas]

Yes we can try this solution.

[aruniverse]

@TomasPetrauskas was @bbastings 's suggestion helpful? Can we close this issue?

[bbastings]

I was actually suggesting that Tomas could create a PR to make this change if it was sufficient for their needs...I don't think what is there now for default args is very useful.