arnoudkooi/sn-scriptsync

Type Overloading and Corrections

MatthewPinheiro opened this issue · 3 comments

There are a few problems with the current typings in the autocomplete folder and/or ways they could be enhanced. Though some of these problems just make the Intellisense less accurate, others raise typescript errors when checkJS is enabled despite being valid ServiceNow code.

Here're the issues I've seen with an example for each, but note that most of these occur more than once in the code. I also can definitely put in a PR to address these if you agree!

  • Unmarked optional params - Methods such as gs.info mark all function arguments as being required when in reality not all of them are (only the first is for gs.info).
  • Multiple function signatures available - Methods like GlideRecord's addQuery have multiple function signatures available (2 args or 3 args) but only one is available in the typings. This can be corrected with an additional function overload (sorry for not reading the contribution.md earlier btw).
  • Nonexistent type - Booleans are referenced as bool instead of the real type which is boolean
  • Incorrect optional/null syntax - An example method under GlideRecord is addJoinQuery(joinTable: string, primaryField: ?, joinTableField: ?) but the question marks used in this way isn't valid TypeScript syntax and those parameters just end up being typed as required any, but I assume it's meant to imply optional params. In other places, I believe I see the question mark used to indicate a potentially null return, but this also isn't valid syntax for that and the null isn't visible for the user.
  • Tuples instead of arrays - In areas like the getParameterNames method for GlideServletRequest, the return is a tuple ([string]) instead of an array (string[]). The difference is that the latter is a dynamically sized list, but the former is a list guaranteed to have a length of 1 (and type errors are raised if you access indices above that length).

The files are generated via file resources/convertTernToDTS.js
So just editing the ts files is not an option.
Maybe with the move to the Monaco editor in platform, there is a better way to reuse the IntelliSense source used there. I'll do some research in the next couple of days. Feel free to also check and share your findings here.

That's a really interesting one. I'm now realizing you said that in my prior PR and I still missed that you said that until now. Sorry!

Here's what I've found poking around a PDI of Utah: The Platform editor retrieves all its typings as a TypeScript parseable string from the following API endpoint:
/api/now/syntax_editor/completions.

This also seems to include all available tables, as well as at least some of the fields available for that table.

This provides all its typings in one place, and while definitely useful, I'll also caution and say this comes with some issues. Due to some combination of syntax errors, tokenization issues, and/or just a massive number of types, the intellisense here just totally loves to hang, not just in the browser platform editor, but even when I try to manipulate it in my own IDE. In some cases, it's also just straight up inaccurate, like documenting that GlideRecord.next() returns a GlideElement instead of a boolean, or still not marking most of the parameters for gs.info as optional.

Still gives some options though.

Good it makes more (intelli)sense now ;)

I found that this call
gs.log( new GlideScriptEditorManager().getApis('sys_script_include', 'script',null));
Now has an extra argument "isMonaco"
gs.log( new GlideScriptEditorManager().getApis('sys_script_include', 'script',null,true));
This returns the types instead of a tern file, so makes my conversion obsolete.

Think this is a good basis and might already solve a few issues you raised.