Implement containerSeemsToBeEmptyDomElement (regex free)

[frodi-karlsson]

Scouring some discussions I see regex is not favored due to Go not being very fast at it (?) so I assume that's why this function got "!!!"d. With that assumption, it's pretty straight forward to convert the Strada implementation to simple string checks so that's this.

[jakebailey]

Hmm, I'm now concerned about this due to #593 (comment) showing that our config parser doesn't actually include "dom" as the output here. Maybe that PR was wrong even though it really seemed right.

[frodi-karlsson]

I see (sort of...), but removing slices.Contains(c.compilerOptions.Lib, "dom") yields no test diff, so I think perhaps this doesn't disprove that comment and this could be wrong

[frodi-karlsson]

I ran a quick test showing that comment to also apply for dom, just to be sure

assert.DeepEqual(t, parsedConfigFileContent.CompilerOptions().Lib, []string{"dom"})

        --- parsedConfigFileContent.CompilerOptions().Lib
        +++ →
          []string{
        -       "lib.dom.d.ts",
        +       "dom",
          }

I wouldn't say that test should be included, since we're already testing the same thing in the compiler tsconfig test

[jakebailey]

If the original code matched only "dom", then I'm just trying to figure out if this means config parsing is still wrong. Not necessarily that your PR was wrong originally.

[frodi-karlsson]

I've revert back to the Strada-like check for now, as a default until we know more there

[jakebailey]

Okay, yeah, so this code in strada has always been broken.

I modified strada to print compilerOptions.lib when this function is called, and the tests print lines like:

[ 'lib.es2015.d.ts', 'lib.es2017.sharedmemory.d.ts' ]
[ 'lib.es5.d.ts', 'lib.dom.d.ts' ]

So !compilerOptions.lib.includes("dom") is always true. 😆

[frodi-karlsson]

Being a bit unsure what to do, I opened an issue here microsoft/TypeScript#61466 and re-implemented the fixed version here

[jakebailey]

I would probably suggest something like:

name := t.symbol.Name

switch name {
case "EventTarget", "Node", "Element":
	return true
}

name, ok := strings.CutPrefix(name, "HTML")
if !ok {
	return false
}

name, ok := strings.CutSuffix(name, "Element")
if !ok {
	return false
}

// ...

[frodi-karlsson]

TIL the cut functions return an ok! Thanks

[jakebailey]

I'd probably just do:

for _, r := range name {
	if !stringutil.IsASCIILetter(r) {
		return false
	}
}

return true

[frodi-karlsson]

I just knew there'd be a util somewhere but I searched for 'a' < and not the other way around, thanks again 😄