CoreMods icon indicating copy to clipboard operation
CoreMods copied to clipboard

Add TypeScript Definitions for JS API

Open ByThePowerOfScience opened this issue 1 year ago • 12 comments

This will allow some minor intellisense for coremod development.

ByThePowerOfScience avatar Oct 31 '24 03:10 ByThePowerOfScience

I'm also interested in adding type support for CoreMods, but I'm going to need a little more information on this since it's rather foreign to me.

  • This looks like an agnostic type support system to me. What IDEs does this support? Ideally I'd like it to be all of them.
  • Does this require CoreMods to apply any additional frameworks?

As of right now, CoreMods uses Nashorn 15.3, which does not include several newer JavaScript features that TypeScript usually relies on, so keep that in mind. I figure this might be better solved by writing JSDocs with type parameters rather than including a TypeScript library file.

Jonathing avatar Oct 31 '24 05:10 Jonathing

I have some limited understanding of this, so answering some of your questions here. Proper clarification/documentation from @ByThePowerOfScience on how to use these type definitions when writing a JS coremod would be greatly appreciated though.

What IDEs does this support? Ideally I'd like it to be all of them.

JavaScript support in the Java IDE ecosystem isn't great. My guess is this'll work well in VS Code and NetBeans, maybe in IntelliJ Ultimate and as for Eclipse, I dunno. Personally I'd be happy even if this only helps Visual Studio Code - some support is better than none. Having a new VS Code window open for writing your JS CoreMods could still be a better experience than no type defs at all.

Does this require CoreMods to apply any additional frameworks?

No, it's a standalone TypeScript type definition file. You can attach it to your IDE to help it provide richer context of your CoreMods JS files. I haven't checked, but I think you could also do a JSDoc @use in your JS to tell it where the type definitions are to have it automatically picked up by your IDE.

As of right now, CoreMods uses Nashorn 15.3, which does not include several newer JavaScript features that TypeScript usually relies on, so keep that in mind.

TypeScript type definitions don't require TypeScript to be used - they can also be used for improved IDE support in plain JS. If using TypeScript, a compilation target set to ES5 would suffice for getting it working on Nashorn 15.3.

PaintNinja avatar Oct 31 '24 12:10 PaintNinja

In that case I'm ok with pulling this, although I do agree that additional clarification from our PR author here would be ideal.

Jonathing avatar Oct 31 '24 12:10 Jonathing

In that case I'm ok with pulling this, although I do agree that additional clarification from our PR author here would be ideal.

It's as @PaintNinja said. Modern JS type checkers - like the native VSCode and IDEA linters - can use these type defintions even for raw JS code of any version.

For example, when validating these definitions, my coremod file looked like this:

/** @type CoreModInitializer */ // JSDoc for the function's return type
function initalizeCoreMod() {
  // ...
}

and IDEA was able to give me proper schema validation for the returned object.

In IDEA Ultimate, my dependencies' coremod JS files were consumed by the typechecker automatically, so I don't doubt the lib.d.ts file would be too.

Further type definitions for the allowed Java classes (Java.type(...)) could be generated using a Gradle plugin like https://github.com/bsorrentino/java2typescript, which generates type stubs for any specified Java classes. This could be used to provide typechecking for the ASM Node classes and ASMAPI as well.

ByThePowerOfScience avatar Oct 31 '24 17:10 ByThePowerOfScience

I have no strong opinion on this, if it makes people's lives easier with minimal maintenance on our end, then i'm fine with it. Would like to see some docs somewhere saying that lib.d.ts is the standard for putting these in. As google tells me that lib.d.ts is typescript's internal file and you an disable it with a flag.

How difficult would it be to integrate the java2typescript plugin in this PR? As might as well do it all in one pass.

LexManos avatar Oct 31 '24 19:10 LexManos

As google tells me that lib.d.ts is typescript's internal file and you an disable it with a flag.

You right; looking at DefinitelyTyped (the gold standard for 3rd party type references), the standard is index.d.ts under a namespace. I'll edit it.

How difficult would it be to integrate the java2typescript plugin in this PR? As might as well do it all in one pass.

~~I mean it's a Gradle plugin........... 😬~~ Nevermind, I was thinking of https://github.com/vojtechhabarta/typescript-generator. I'll take a look.

EDIT: Nevermind, neither work for what we need. One only generates method stubs, the other only generates fields. I don't have the knowledge needed to combine the two.

ByThePowerOfScience avatar Oct 31 '24 19:10 ByThePowerOfScience

Ok I got it, but it's janky, manual, and it doesn't have the fields. I wouldn't recommend taking that commit; it'll probably be more confusing than just referencing the classes manually.

Either way, that part is far less necessary than the basic coremod schema being available somewhere.

ByThePowerOfScience avatar Oct 31 '24 21:10 ByThePowerOfScience

Since the work needed to make this efficient is a bit much at the moment, I'm not considering it an urgent pull for CoreMods 5.2.0.

Don't get me wrong though, I absolutely do want better IDE support for CoreMods. But if I pull this as-is, it will make maintainability a nightmare. A group of ASMAPI-related PRs which were recently merged have already slightly outdated the content in this PR.

Jonathing avatar Nov 02 '24 17:11 Jonathing

Don't get me wrong though, I absolutely do want better IDE support for CoreMods. But if I pull this as-is, it will make maintainability a nightmare. A group of ASMAPI-related PRs which were recently merged have already slightly outdated the content in this PR.

We can just remove the ASMAPI part then. The thing that's absolutely required is including a schema for the expected return value of initializeCoreMods.

ByThePowerOfScience avatar Nov 09 '24 18:11 ByThePowerOfScience

Ok, then let's just remove the ASMAPI part for now then. I don't expect the schema for initializeCoreMods to change for a while, if at all.

Jonathing avatar Nov 10 '24 04:11 Jonathing

As mentioned in MinecraftForge#10177, we are planning on deprecating CoreMods in the (somewhat?) near future. I would still like to make one last minor patch, 5.3, that includes the changes you've made here, but know that there probably won't be any changes to the implementation aside from any last-minute additions to ASMAPI. If you have the time, please cleanup this PR (basically just remove the ASMAPI stuff if you have it) and let me know so I can review it and have it merged in soon.

Jonathing avatar Nov 18 '24 22:11 Jonathing

This PR needs to be rebased.

Jonathing avatar Jan 14 '25 23:01 Jonathing