Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

branch: typescript #360

Closed
samreid opened this issue Aug 28, 2021 · 5 comments
Closed

branch: typescript #360

samreid opened this issue Aug 28, 2021 · 5 comments

Comments

@samreid
Copy link
Member

samreid commented Aug 28, 2021

From phetsims/chipper#1069

@zepumph suggested creating a typescript branch for axon and scenery, so at the moment we decide to enable the typescript build chain for all sims, we can immediately get the benefits in common code.

@samreid
Copy link
Member Author

samreid commented Aug 30, 2021

I wrote a draft of Property, but there are some caveats.

  • axon and tandem are combined since they have cross references, so they share a tsconfig at the moment
  • the tsconfig doesn't include the tests, since the tests include downstream dependencies
  • so I disabled precommit hooks to commit.
  • the "typescript" branch of axon should not be used with the other typescript branches, because it is for a common repo. Other typescript repos should not depend on this yet, until we have a process and are at that phase.

samreid added a commit that referenced this issue Aug 30, 2021
samreid added a commit that referenced this issue Aug 30, 2021
@samreid
Copy link
Member Author

samreid commented Aug 30, 2021

New idea about how to temporarily use generic Property in typescript sims without having to turn on the typescript build chain for all sims: generate d.ts files from the axon/typescript branch and check the d.ts files in to master.

Pros:

  • Enables type safety/autocomplete/navigate for TypeScript projects
  • Does not require changing *.js to *.ts in common code

Cons:

  • This makes developing axon/typescript more difficult since you have to generate d.ts and move it to master each time
  • CMD+B in Java no longer takes you to the *.js file--it now takes you to the d.ts file. There are other navigation methods, but none of them work quite as well as *.js navigating without the d.ts file.

This may be a good strategy until we enable typescript for all repos, if developers are OK with the tradeoffs in the cons section.

@samreid
Copy link
Member Author

samreid commented Oct 12, 2021

While I was working on phetsims/bending-light#401, I noticed there is a JSDoc annotation for template variables in JavaScript, described here: https://stackoverflow.com/questions/16017627/document-generic-type-parameters-in-jsdoc

When I applied this new JSDoc to Property, TypeScript was able to read and understand it. So that seems like a preferable solution until we approve TypeScript in common code. I tested JavaScript usages in Fourier and they didn't seem disturbed. I'll commit this to master shortly.

samreid added a commit to phetsims/bending-light that referenced this issue Oct 12, 2021
samreid added a commit to phetsims/gravity-and-orbits that referenced this issue Oct 12, 2021
@samreid
Copy link
Member Author

samreid commented Oct 12, 2021

I was able to use the same JSDoc template pattern for Emitter, supporting an array of types. It's working really well.

    const e0 = new Emitter<[]>();
    e0.emit(); // ok
    e0.emit( 'testing' ); // bad
    e0.addListener( () => {} ); // ok
    e0.addListener( ( age: number ) => {} ); // bad

    const e1 = new Emitter<[ boolean ]>();
    e1.emit( true ); // ok
    e1.emit( 'hello' ); // bad
    e1.addListener( ( enabled: boolean ) => {} ); // ok
    e1.addListener( ( age: number ) => {} ); // bad

    const e2 = new Emitter<[ string, number ]>();
    e2.emit( 'bye', 7 ); // ok
    e2.emit( 'null' ) // bad
    e2.addListener( ( name: string, age: number ) => {} ); // ok
    e2.addListener( ( name: string, age: number, time: Date ) => {} ); //bad

image

@samreid
Copy link
Member Author

samreid commented Nov 19, 2021

Branch deleted, closing.

@samreid samreid closed this as completed Nov 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant