Improve MainModuleFactory type emitted by create_tsd

[s-h-a-d-o-w]

trafficstars

This is in accordance with the type on DefinitelyTyped.

[sbc100]

Looks you likely need to run some tests with --rebaseline

[s-h-a-d-o-w]

Whoops, should've thought about there being tests for this. Sorry about that.

Since I will need to set up the whole toolchain to run that, it's going to take a bit though.

[sbc100]

The way I recommend doing it is emsdk install tot && emsdk activate tot then export EM_CONFIG=/path/to/emsdk/.emscripten

[s-h-a-d-o-w]

Thanks but I'm on windows, without LLVM or clang. :sweat_smile:

[sbc100]

Thanks but I'm on windows, without LLVM or clang. 😅

emsdk install tot takes care of installing all that for you at the correct version.

[sbc100]

I'm happy to do that rebase for you if install emsdk is too much? (But how are you running emscripten then?

[s-h-a-d-o-w]

Nah, it was just a oversight - I forgot to set the env variable like you told me to. It works great!

[s-h-a-d-o-w]

So... running with --rebaseline logs nothing to the console and changes no files. It also finishes running very quickly. Should it?

I'm not confident that everything works as intended because when I run random tests, many fail, e.g.:

  File "C:\Users\andy\temp\emsdk\python\3.9.2-nuget_64bit\lib\unittest\case.py", line 670, in fail
    raise self.failureException(msg)
AssertionError: either d8 or node >= 24 required to run wasm64 tests.  Use EMTEST_SKIP_WASM64 to skip

Which is interesting given that "Current" node is 23.

Anyway... I only stumbled across this randomly and usually don't use emscripten, so it's probably easiest for both of us if you run that. Thanks for offering that!

[sbc100]

Actually I was chatting with @brendandahl and we think maybe the DefinitelyTyped definition is wrong here. We really want something that is a object/dictionary that matches INCOMING_MODULE_JS_API.

[sbc100]

We can maybe land this for now then move away from it.

BTW, are using using the generated types in combination with DefinitelyTyped somehow? Should we try to update to delete what is stored in DefinitelyTyped?

[s-h-a-d-o-w]

No but thanks for asking!

(Long story: I arrived here from trying to improve the tree sitter TypeScript types, which are currently not in the best shape. I stumbled across this PR and thought that maybe it didn't get any attention because of the missing PR description or maybe because it didn't address the problem at the root, so I tried to. I saw that the factory type in DefinitelyType has existed for at least 5 years and was discussed 5 years ago, so assumed that it was correct. But then I realized that vemoo was right and the types in tree sitter weren't actually generated using emscripten. And that my problem was much simpler anyway and the type problems in tree sitter don't affect me. So I decided to leave these problems be and refocus on my own work.)

[sbc100]

@brendandahl should file a bug to have https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/emscripten/index.d.ts removed? It seems like emscripten-generated types via --emit-tsc are always going to better, and anything checked into DefinitelyTyped likely never going to be up-to-date or match the interface of any specific emscripten output.

[brendandahl]

@brendandahl should file a bug to have https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/emscripten/index.d.ts removed? It seems like emscripten-generated types via --emit-tsc are always going to better, and anything checked into DefinitelyTyped likely never going to be up-to-date or match the interface of any specific emscripten output.

Yeah, I can do that. It looks like that file as some better typing information for a few interfaces that we could pull over and put in the JS doc comments for the library (which will then generate TS definitions).