Hide type models for Ruby by default

[koesie10]

This is a first step into making type models hidden for non-canary users. To test this, you'll need to change extensions/ql-vscode/src/model-editor/languages/ruby/index.ts to make isHidden: () => true since you can't open the Ruby model editor without canary.

Currently, you will still see that there are unsaved models when the type models get generated on start up. This will be fixed in a follow-up PR.

Checklist

• CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
• Issues have been created for any UI or other user-facing changes made by this pull request.
• [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

[charisk]

Does it make sense to separate what we show in the UI vs what we store in memory/on-disk? It seems a bit strange to be going so far down the stack before deciding something is hidden + be passing the canary flag all the way down.

[koesie10]

Does it make sense to separate what we show in the UI vs what we store in memory/on-disk? It seems a bit strange to be going so far down the stack before deciding something is hidden + be passing the canary flag all the way down.

Yes, I think that makes sense. The problem is that we still need to pass down all of this information to the ModelTypeDropdown since users shouldn't be able to select the Type models for Ruby in non-canary mode, so it would only reduce the code in here. It would also be problematic when loading existing model files which do contain type models: we would need to keep track of these models and ensure that they are not overridden, so we would still need to keep them in memory.

I'm working on another PR to at least move the generated type models in non-canary mode to a separate file that we won't be keeping in memory. That means that this code will be less likely to be hit, but it's still necessary for it to be there.

[charisk]

Ok thanks. I think this is okay considering it's a temporary thing as well (I'm assuming!).

[koesie10]

Ok thanks. I think this is okay considering it's a temporary thing as well (I'm assuming!).

I don't think this will be temporary: the intention is to permanently hide type models from users and always generate them. I would like to find a better solution for this, but couldn't really come up with one. An alternative would be to pass in something like hiddenPredicates into the view state and pass this down, but that would just change what data is passed down.

[charisk]

Ok thanks. I think this is okay considering it's a temporary thing as well (I'm assuming!).

I don't think this will be temporary: the intention is to permanently hide type models from users and always generate them. I would like to find a better solution for this, but couldn't really come up with one. An alternative would be to pass in something like hiddenPredicates into the view state and pass this down, but that would just change what data is passed down.

The canary part is temporary though right?

[koesie10]

The canary part is temporary though right?

No, the intention is for users running in canary mode to be able to add and edit type models, and regular users not in canary mode should not need to know/care about type models. So, in non-canary mode we will automatically generate type models to a separate file and not allow users to view/edit/add type models, and in canary mode we will not generate type models automatically and allow users to view/edit/add type models. We might move this to a separate setting in the future rather than basing it on canary mode, but I don't think that would fundamentally change that we need to pass through the value of a setting.

[charisk]

The canary part is temporary though right?

No, the intention is for users running in canary mode to be able to add and edit type models, and regular users not in canary mode should not need to know/care about type models. So, in non-canary mode we will automatically generate type models to a separate file and not allow users to view/edit/add type models, and in canary mode we will not generate type models automatically and allow users to view/edit/add type models. We might move this to a separate setting in the future rather than basing it on canary mode, but I don't think that would fundamentally change that we need to pass through the value of a setting.

This does feel like a separate setting to me then - we want to have a way to flip between automatically generating types and not, for different users. I see canary as a way to gradually ship a feature to users, not to control the feature set long-term.

Based on that, I would have shouldGenerateTypeModels setting (or something like that) and pass that in the React views rather than isCanary. For now we could control its value based on isCanary, but ideally we'd move to a more proper setting eventually. Appreciate this doesn't remove the need to prop drill.

That's just an opinion though and I'm happy to be convinced otherwise, or even go with what you've got for now, if it's blocking you.

[koesie10]

@charisk Thank! I definitely agree, so I'll move this to codeQL.model.showTypeModels.

[charisk]

LGTM!