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

Let us slowly move to Typescript. (instructions inside) #4434

Open
ghost opened this issue Oct 14, 2016 · 13 comments
Open

Let us slowly move to Typescript. (instructions inside) #4434

ghost opened this issue Oct 14, 2016 · 13 comments

Comments

@ghost
Copy link

ghost commented Oct 14, 2016

I set up Typescript pretty easily with Cesium (with minimal friction).

  1. Install Typescript:
    npm install --save-dev typescript

  2. Create a file in the root directory tsconfig.json:

    {
        "compilerOptions": {
            "target": "es3",
            "module": "amd"
        },
        "include": [
            "./Source/**/*.ts"
        ]
    }
    
  3. Rename all js files in the pattern below (except the ones with "!") to .ts (taken from gulpfile.js):

    ['Source/**/*.js',
    '!Source/*.js',
    '!Source/Workers/**',
    '!Source/ThirdParty/Workers/**',
    'Source/Workers/createTaskProcessorWorker.js'];
    

    I just went in the directories (not the "!" ones) and ran the recursive command:

    find . -name "*.js" -exec rename 's/\.js$/.ts/' '{}' \;
    
  4. Use the Typescript watcher to automatically compile Typescript to Javascript upon changes:

    tsc -w -p .
    
  5. The resulting Javascript files will be placed next to the ts files. This will let us keep our current build process and reap the benefits of Typescript with minimal effort. We can just use a .gitignore for the js files. gulp-typescript does exist (see link at bottom).

Remember that Typescript is a superset of Javascript. Therefore, all of our current code works completely fine. Now, we can incrementally migrate. With type-checking, compiler options like strictNullChecks, noImplicitAny, noImplicitReturns, and much more (see link below), a proper module system with import/export, we can create new features faster and find dormant bugs (with type checking). Also, less tests! We don't have many contributors... so Typescript would definitely help. And you can finally use a proper IDE with autocomplete and refactoring tools.

We can complicate the build process with gulp-typescript and more, but I want us to start with the bare minimum first so we don't scare anyone away. Minimal friction is the keyword here.

@ghost ghost changed the title Let's slowly move to Typescript. (instructions inside) Let us slowly move to Typescript. (instructions inside) Oct 14, 2016
@pjcozzi pjcozzi added the 2.0 label Oct 15, 2016
@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 15, 2016

@thinkpadder1 thanks for the notes on Typescript. In the future, we would prefer that you start a discussion on the forum for something like this. Given its incredible impact, it will require significant discussion. For now, we'll have to table the discussion as we are heads down on 3D Tiles, but I have labeled this 2.0 so it can be considered when we make bigger module changes.

In the meantime, you might want to research how this would affect Node.js users of Cesium and how the transpiler would impact development; the core dev team will insist on a smooth dev experience with virtually no overhead.

@mramato
Copy link
Contributor

mramato commented Oct 18, 2016

Thanks for the list of resources and getting started details @thinkpadder1. I would love to move to something like TypeScript and I know @kring is a bing proponent as well. It will be a non-trivial effort and I agree with @pjcozzi isn't not something that can happen until the 3D Tiles work is done (given the amount of new code that that branch will introduce and the amount of time to get the team used to working in TypeScript).

It's definitely something I would start prototyping at a hackathon or when I suddenly have free time if no one beats me to it. My main concerns are testing, coverage, performance, and backwards compatibility with existing Cesium applications. I also wouldn't mind surveying the JS landscape and see what possible benefits TypeScript would provide over other approaches, like Babble and flow. I'm fairly confident that TypeScript will win out here, but we need to do our due diligence.

@kring
Copy link
Member

kring commented Oct 19, 2016

I spent a bunch of time looking at using TypeScript for TerriaJS. The main problem with it was the ecosystem. You can't use jsdoc or eslint, for example. Similar tools for TypeScript are far less mature. In many cases, tools that initially looked promising were expecting older versions of TypeScript and hadn't been updated in many months. I even tried ugly hacks like having the TS compiler generate JavaScript with comments and then running that through jsdoc, but I couldn't get it to work reliably.

This was probably 8 months ago now, so maybe things have improved.

Unfortunately, IMO, the ecosystem matters just as much if not more than the language. In the end we ended up going with ES2015 and Babel. We haven't really tried Flow yet, but it looks pretty good.

@mramato
Copy link
Contributor

mramato commented Oct 19, 2016

Thanks @kring those were exactly the types of problems I was worried about. Even if things have improved, it shows that this is actually a major change for our code and definitely will require lots of thought and planning.

As an aside, since you went with 2015 and Babel, are you using modules? How does testing and coverage get handled with transcompilation?

@ghost
Copy link
Author

ghost commented Oct 19, 2016

@kring For linting, see TSLint.
If you really need ESLint, the ESLint team is working on it (experimental): typescript-eslint-parser.

Typescript supports JSDoc style comments and works great in Visual Studio Code. Even if you don't use Typescript, it's a great editor with the Salsa service.

There are even extensions that will generate most of the JSDoc for you: Document This

Here is a list of all currently supported JSDoc annotations (which is being actively worked on).

Lastly, someone is working on a JSDoc to Typescript Definitions plugin and he is targeting Cesium + OpenLayers: jsdoc-typescript-plugin. It could be the middle ground between sticking with what Cesium has and fully committing to Typescript (at the bare minimum, it would help people using Cesium).

@kring
Copy link
Member

kring commented Oct 19, 2016

@thinkpadder1 yeah I had previously discovered TSLint, as well as typescript-eslint-parser. It looks like both have made some progress since I've looked, but still aren't nearly mature as ESLint with Babel.

re: doc, those things are all nice, but they don't help us generate the documentation on cesiumjs.org: http://cesiumjs.org/refdoc.html

@ghost
Copy link
Author

ghost commented Nov 22, 2016

@kring Have you guys looked at TypeDoc?

Cloudflare uses it: Generating Documentation for TypeScript Projects

The post shows how to use it and compares it against JSDoc.

@custompro12
Copy link

Any update on this?

@mramato
Copy link
Contributor

mramato commented Jun 3, 2020

As of Cesium 1.70.0, we are shipping official TypeScript definitions. If you want to read all of the gory details, and potential future plans for TypeScript and CesiumJS, check out this blog post: https://cesium.com/blog/2020/06/01/cesiumjs-tsd/

Sometime over the next week or so, I'll be consolidating any TypeScript issues into a new master issue for plans going forward.

Thanks!

@bampakoa
Copy link
Contributor

bampakoa commented Jun 3, 2020

@mramato I think that you meant we are shipping 😄

@mramato
Copy link
Contributor

mramato commented Jun 3, 2020

Whoops, thanks. It was supposed to say "now shipping" fixed 😄

@jjspace
Copy link
Contributor

jjspace commented Jan 9, 2025

I keep running into annoyances or "pain points" when working in CesiumJS in VSCode with regards to typings. Intellisense relies on the TS Server but our JSDoc reliant setup gives it many issues.

In no particular order, an incomplete list of recent pain points I've noticed with types:

  • "sidecar" types files like what we have for defined, Check, and I want for defaultValue
    • These are super helpful for local development with things like type assertions but tsd-jsdoc can't import them so it gets messy
    • Switching to a different way to generate our .d.ts files may eliminate this, see comment Update or remove tsd-jsdoc #10455 (comment)
  • Cannot "nest" values in JSDoc and TS
    • See Resource, Resource.ConstructorOptions and Resource.RetryCallback, they're "valid" for JSDoc and generate the docs we want all nested on the Resource doc page but are NOT valid for TS so you get errors like Namespace 'Resource' has no exported member 'RetryCallback'.ts(2694)
  • private is too restrictive. We use it to hide things from the public docs but this has the side-effect of the TS server thinking they are inaccessible
    • How do we say "private = internal to the library" instead of truly private to the class?
    • This is probably a documentation issue not a typing issue. Things should not be set as private but should be excluded from the docs, maybe another JSDoc tag can be added
    • Even when it is actually "private to a class" the outdated way that we construct classes on functions and object prototypes makes the TS server not realize we are accessing it "within the class"
      • Resource.prototype.fetch demonstrates this, _makeRequest is @private so it says Property '_makeRequest' is private and only accessible within class 'Resource'.ts(2341) even though we are inside the Resource class
  • Enums in general don't work for TS
    • If we use Object.freeze on the export but not the initialization TS will think it's a Readonly<Type> type instead of just a number or string and then show errors when comparing values
    • Also some of our enums still attach functions to the enum which makes the object a mixed type which is a big confusion for TS and not as easy a fix of just moving where we call Object.freeze
  • TS (and thus intellisense) cannot at all understand how to connect Class properties when we define them using Object.defineProperties(Class.prototype), especially when we only define a getter or setter

If you want to see more just turn on Typechecking in VSCode for JS with the js/ts.implicitProjectConfig.checkJs setting and look at pretty much any file in the codebase and see all the errors (then try to fix them and get frustrated 😅)
(I may continue to update this as I think of more)

CC @ggetz

@javagl
Copy link
Contributor

javagl commented Jan 15, 2025

private is too restrictive. We use it to hide things from the public docs but this has the side-effect of the TS server thinking they are inaccessible. How do we say "private = internal to the library" instead of truly private to the class?

I'd like to disagree here. In fact, I've previously been advocating for an access modifier that is stronger than private.

(Originally, this referred to Java. There, the question about the "internal to the library" is handled more cleanly, with the "default" visibility: When something is neither public nor protected nor private, then it is visible within the package. But this is still discouraged. There are very few cases where a single access modifier is sufficient. You don't want a public method, at all. It should basically always be public final. More on that: http://wiki.apidesign.org/wiki/ClarityOfAccessModifiers . Unfortunately, all this isn't really applicable to JavaScript/TypeScript on the language level. But that Wiki entry may raise awareness and serve as a guideline).

Now, what should be the purpose of "stronger than private"?

Using some random code snippets from some class in CesiumJS (the context doesn't matter):

  boundingVolume: {
    get: function () {
      return this._boundingVolume;
    },
  },

  boundingSphere: {
    get: function () {
      return this._boundingVolume.boundingSphere;
    },
  },

Nice and straightforward: You can call
const v = thing.boundingVolume;
or
const s = thing.boundingSphere;

I could now mention that the caller could, in fact, just write
const v = thing._boundingVolume;
and say "I'm within 'the library', I can do this".

Beyond that: You don't know that these lines are not just propery accesses, but actually calling 'getters'. That 'getter' could do anything. At the call site, you have zero chance of knowing what this line does, and (worse) zero chance of figuring that out (except for a debugger run...).

So much about language bashing 😁
Coming back to "stronger than private:

Imagine that this 'getter' actually does more than just returning the property. Imagine that it is refactored to something like this:

  boundingVolume: {
    get: function () {
      if (this._boundingVolumeWasModified ) {
        this._boundingVolume = update();
        this._boundingVolumeWasModified = false;
      }
      return this._boundingVolume;
    },
  },

A pretty common "dirty flag" was introduced. Fortunately, this change is hidden from the users. Callers who did
const v = thing.boundingVolume;
will always receive an up-to-date bounding volume. And yes, those who did
const v = thing._boundingVolume;
thinking that "They are in the library, so they can do this" are now doomed, because they receive the outdated bounding volume, but ... let's track that in an issue.

But the point is that even within the class, there is an inconsistency now: The boundingSphere getter also uses that outdated bounding volume!

Some pseudo-spec here could be:

const s0 = thing.boundingSphere;
const s1 = thing.boundingVolume.boundingSphere;
assertEquals(s0, s1);

and this one would break now.

If the getter was implemented as

  boundingSphere: {
    get: function () {
      // NO: return this._boundingVolume.boundingSphere;
      // YES, use the 'boundingVolume' getter:
      return this.boundingVolume.boundingSphere;
    },
  },

then the state would remain consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants