Dialog methods don't allow you to assign subtypes

[BeksOmega]

Check for duplicates

• I have searched for similar issues before opening a new one.

Description

The dialog methods (setPrompt, setAlert, setConfirm) don't allow you to assign subtype functions to those values, because of how they are being called.

For example, you can't provide a prompt function that has additional optional parameters, even though it is a subtype of the normal prompt function type, because we wrap the provided function in another function call.

export function prompt(
  message: string,
  defaultValue: string,
  callback: (p1: string | null) => void,
) {
  promptImplementation(message, defaultValue, callback);
}

As you can see, prompt wraps promptImplementation, so any extra params passed to prompt don't get passed to promptImplementation.

Suggested solution

Instead of assigning to promptImplementation we can assign to prompt directly.

I think this should work, but I might be missing something. Please let me know!

[maribethb]

I think your solution is fine (I'm not sure why we have promptImplementation separated out but maybe for type reasons?

But I don't understand why this is needed. Either Blockly is the one calling prompt, in which case it's not going to pass any extra parameters, or the developer is calling their own prompt, which they could just do directly.

Can you clarify? Also, is there any need to prioritize this soon? If there is a good use case we can leave this as "help wanted" unless a partner has asked for this soon or something.

[BeksOmega]

I think your solution is fine (I'm not sure why we have promptImplementation separated out but maybe for type reasons?

But I don't understand why this is needed. Either Blockly is the one calling prompt, in which case it's not going to pass any extra parameters, or the developer is calling their own prompt, which they could just do directly.

Can you clarify? Also, is there any need to prioritize this soon? If there is a good use case we can leave this as "help wanted" unless a partner has asked for this soon or something.

This is a blocker for spork block related things. Their prompts get created from within spork-blocks as well as spork-UI. I had the same thing about "just call your custom thing customly" but the cross-repo usage means it needs to live in the global namespace. So while there are other places one could put it, this just kinda makes the most sense, and is a relatively simple type change for us. I think I already have the code in a branch, I just didn't want to put it up until there had been discussion.

[maribethb]

the cross-repo usage means it needs to live in the global namespace. So while there are other places one could put it, this just kinda makes the most sense, and is a relatively simple type change for us.

In general, I don't think this is a very strong argument. So Scratch/CSF has created something they need to access from more than one repo. That is their own dependency problem to solve, and we shouldn't host global state just because we're a convenient place to do so.

That being said, this is a small change and I don't see a reason for it to be written the way it currently is, so I'm not fundamentally opposed to it. I just don't want to use this as justification in the future next time we're asked to hold some global state we don't need.

[maribethb]

@BeksOmega you can go ahead with this change!

[BeksOmega]

PR that fixed this was reverted. We're going to try to fix this for Scratch a different way.