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

Typescript support #6

Open
brianmhunt opened this issue Sep 18, 2016 · 21 comments
Open

Typescript support #6

brianmhunt opened this issue Sep 18, 2016 · 21 comments

Comments

@brianmhunt
Copy link
Member

From #1 , just so I can clean up relevant issues there.


IanYates commented on Jul 6
It really depends on whether we're talking about writing the library in TS or just making it friendly to consume from TS.

The latter is easy - we just need to make a type definition. If this library stays pretty close to KO in terms of a public API surface then it'll have a very similar definition file. I'd be happy to put something together as this takes shape.

If you're talking about writing the library in TS then that's a much bigger discussion. I personally really like it 😍 but I can understand that many don't.


brianmhunt commented on Jul 6
Consumption will be the goal since I don't know TS. :)

I wonder if there's a way to automate a type definition. What do they look like?
@IanYates


IanYates commented on Jul 6
Automate.. Not that I'm aware of.

Here are some definitions of things with which you're rather familiar

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/cc3d223a946f661eff871787edeb0fcb8f0db156/knockout-secure-binding/knockout-secure-binding.d.ts
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/cc3d223a946f661eff871787edeb0fcb8f0db156/knockout.amd.helpers/knockout-amd-helpers.d.ts
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/cc3d223a946f661eff871787edeb0fcb8f0db156/knockout.postbox/knockout-postbox.d.ts
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/0b20db14dc8d546bd6e8e7b3bcaaf2289f63b55d/knockout.es5/knockout.es5.d.ts
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/cc3b657f2655c8c7cd290ff83dc052be637d8fe2/knockout/knockout.d.ts
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/17fe51d7edd2aab8917d6868b8b69e6014043559/knockout.punches/knockout.punches.d.ts

Also see http://www.typescriptlang.org/docs/handbook/knockout.html for an intro to TS using KO
and
http://www.typescriptlang.org/docs/handbook/writing-declaration-files.html for info about definition files straight from the horse's mouth
👍 2

@cervengoc
Copy link
Contributor

Since TS 2, the best and most convenient way for handling typings is that each library provides its own d.ts files. This way there are no version conflicts, and the typings are coming together with the library itself in the npm package for example.

So I +1 this, I think it's definitely important to provide a typing file.

@codymullins
Copy link
Contributor

codymullins commented Jan 7, 2017

@brianmhunt let me know if you need a hand with this one.

@brianmhunt
Copy link
Member Author

@codymullins Thanks -- is what you need to create a .ts file in the index.js of this repo?

@cervengoc
Copy link
Contributor

The current recommendations to define the typings for a library could be roughly summarized like this.

  • Each NPM package should have it's own typings file placed in the root, named as index.d.ts, or anythings else (but index.d.ts is the default search path).
  • The package.json should contain a root level setting called types, and it should point to the .d.ts file of the library. If it's index.d.ts, then this configuration can be omitted.
  • If the library supports UMD, the typings file itself should folllow the TypeSciprt 2.0 recommendations for UMD library typing files. I'm actually in the progress of updating the knockout typings to meet these points, so it can be a good starting point when it's ready (in the next weeks sometime).

Hope it helps.

@codymullins
Copy link
Contributor

@cervengoc so are you already working on the typings file for tko or only for Knockout? Are you basing it off the DefinitelyTyped typings file?

@cervengoc
Copy link
Contributor

cervengoc commented Jan 7, 2017

I'm working on knockout, not tko, and yes, I'm actually refactoring the latest DefinitelyTyped version to match the TS2 recommendations.

The typings for tko will be a bit trickier as we have to break the typings as well according to the package, so that we have one typings file per package.

@codymullins
Copy link
Contributor

@cervengoc, I'm not familiar yet with how TKO is broken out - I see there are multiple packages and looks like there's one base repo that's used for the current build? In that case you're saying each package would need a typings file, correct?

I'd be interested to see a PR or where you're working on the typings, once you are done we could probably work to use that as a base for TKO in order to stay consistent.

@brianmhunt
Copy link
Member Author

Ideally I think, if it's possible, the Typescript definitions ought to be in the leaves.

That way, when someone decides to mix-and-match with other packages that follow the "tko formula" the definitions will be automatically included in the build project.

We'll probably have to come up with a way to build the descriptions from all the leafs (tko.utils, tko.observable, etc.) into one description for the end-result (tko or ko), but I think this is the correct long-term design.

... I'm not sure if or how that's possible, but I thought to mention it.

@caseyWebb
Copy link
Contributor

As of #39, TKO is ready for incremental adoption of TypeScript in the repo itself. If there is interest in migrating the existing codebase, I'm interested in helping out.

@codymullins
Copy link
Contributor

Me too, I'm down to help migrating over.

@Sebazzz
Copy link

Sebazzz commented Apr 20, 2018

Yes, I am also happy to help.

@brianmhunt
Copy link
Member Author

Awesome. There's quite a bit of work on this in main Knockout, which we'll want to remain compatible with. Feel free to break out a PR for this.

@Sebazzz
Copy link

Sebazzz commented Apr 23, 2018

How is the dependency graph between the sub packages? The easiest way to introduce Typescript is to start with the core package, then work the way up from that.

@brianmhunt
Copy link
Member Author

brianmhunt commented Apr 23, 2018

The graph looks something like this, from packages/:

  • tko (or knockout)
    • tko.binding.*
      • tko.bind
    • tko.computed
      • tko.observable
    • tko.provider.*
      • tko.provider

Various utilities from tko.utils(.*) are imported all over the place.

I've attached the rollup visual.html for packages/tko (packages/knockout is nearly identical), which can aid in understanding the build.

visual.zip

@Sebazzz
Copy link

Sebazzz commented Apr 23, 2018

(your visual.zip appears to be corrupted and cannot be opened)

OK, then the most logical place to start is with tko.utils, after that it it can probably be done more concurrently the further we go.

@brianmhunt
Copy link
Member Author

Thanks @Sebazzz ; I've updated visual.zip, hopefully it'll work now.

@Sebazzz
Copy link

Sebazzz commented Apr 24, 2018

Thanks. I am working on tko.utils now. I suggest other contributors wait until we have that one merged in. There are still some loose ends to tie up, and the first pull request can be used as discussion piece to determine some technical choices going forward.

(I already found the first bug using Typescript)

@caseyWebb
Copy link
Contributor

caseyWebb commented Mar 15, 2019

@brianmhunt reached out to me for help in getting TKO on the path to TypeScript, so I've been playing around with the repo's build process and I think I've finally done enough discovery to develop a reasonable gameplan.

With only very minor tweaks to the current build process, we can fully support transpiling TS files (and supporting index.ts entry files), however, in order to not have to do this wholesale, compilerOptions.allowJS must be true in the tsconfig, which precludes using the new project references in tsc, which IMO is going to be the absolute easiest way to accomplish transpilation and emit declarations & source-maps.

So, I suggest making those tweaks, and then beginning the process of actually rewriting/refactoring into TypeScript, while still relying on the hand-written declaration file currently used in publishing until the entire codebase is in TS.

At which point, I propose moving to a tsc (w/ composite/project references) + webpack build process and producing 3 builds:

default

  • module entry in package.json
  • ES5
  • ESM
  • used by webpack and other bundlers

universal/bundle

  • main entry in package.json
  • ES5
  • bundled w/ UMD wrapper
  • used in Node envs (jest, other test runners) & in the browser

esnext

  • nonstandard esnext entry in package.json
  • ES2017 (or whatever runs in the latest Chrome/Firefox natively)
  • ESM
  • used by webpack and other bundlers (with configuration)

As well as declarations, declaration maps, and source maps for debugging pleasure.

I've got no problem tackling the tooling; I've done a lot of work in that area.

@brianmhunt
Copy link
Member Author

@caseyWebb This sounds like the right choice, thank you for spearheading this (and thank you @Sebazzz for getting the initial work started on Typescript-ification).

What do you propose for next steps @caseyWebb? I'll try out the build on the #92 in the browser, and I'll report results to that PR.

@caseyWebb
Copy link
Contributor

caseyWebb commented Mar 15, 2019

I think at this point the best thing is just going to be brute force; pick a package and start translating.

This should be the order (I believe) with regards to transitive dependencies:

@tko/utils.functionrewrite
@tko/utils.jsx
@tko/utils
@tko/observable
@tko/provider
@tko/computed
@tko/filter.punches
@tko/provider.attr
@tko/provider.multi
@tko/provider.native
@tko/lifecycle
@tko/bind
@tko/binding.core
@tko/binding.foreach
@tko/binding.if
@tko/binding.template
@tko/utils.parser
@tko/builder
@tko/provider.bindingstring
@tko/provider.mustache
@tko/provider.databind
@tko/provider.virtual
@tko/utils.component
@tko/binding.component
@tko/provider.component
@tko/build.knockout
@tko/build.reference

For converting each package, the approach I'd take is:

  • cd into the directory
  • git mv each file (git mv index.js index.ts)
  • commit
  • disable strict in tsconfig.json
  • fix compiler issues
  • commit (excluding tsconfig.json)
  • re-enable strict
  • fix compiler issues
  • commit

Organizationally, I suggest using either my branch (caseyWebb/tko#typescriptify) or one opened on knockout/tko and give access to anyone willing to contribute. I'm usually on Gitter during the day and can help out with most TS issues.

My next steps are getting Karma working and doing a code review of the work on tko.utils

@brianmhunt
Copy link
Member Author

@caseyWebb Fantastic. For my next steps, I'll look into typescripting utils.jsx, and I'll PR to your branch.

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

5 participants