Typescript) fix errors #202

phillipc · 2025-01-12T17:18:19Z

As announced, we have started to make TKO more type-safe. To get a startpoint, we have set the tsconfig as softly as possible and continue to allow "any" types.

We (@Auge19 , @mcselle ) validate the typescript code with tsc. The current status is only 313 errors left. We began with 7410 errors. With this draft PR we would like to get feedback and start our own review. The goal is 0 errors with tsc. If this PR is ready and merged, we would like to continue with ESLINT.

All tests continue to pass.

add some types

#we should remove jasmine..

switch d.ts-version for jasmine (the project used jasmine 1.3)

1) [] => new Array 2) childNodes => children 3) extend some global type-defs

…_typed

fix wrong jasmine imports fix method signature

…ong_typed

…yped

phillipc · 2025-02-14T14:12:54Z

In the last few days we have done some more tests and can now officially submit our PR for approval. We hope to get closer to a release of TKO 4.0. I think we are ready for a production release!

brianmhunt

Looking good so far, I'm not done reviewing about 1/10th of the way, but wanted to add some interim comments and I'll continue to come back to this as time permits.

Thank you!

brianmhunt · 2025-02-21T21:53:04Z

README.md


-Note that running `karma` or `rollup` will create a `visual.html` file that shows the proportional size of imports into each package.
+Note that running `karma` will create a `visual.html` file that shows the proportional size of imports into each package.


I think there's an equivalent with esbuild [1] if we wanted to re-enable this

[1] https://esbuild.github.io/analyze/

that's cool, I didn't know that before

brianmhunt · 2025-02-21T21:54:34Z

global.d.ts

+/// <reference types="jasmine" />
+/// <reference types="jquery" />
+
+export { };


what's the export for?

(nice to add a comment to document its purpose)

export {} allows redefining/type-merging the window module in d.ts files, see https://bobbyhadz.com/blog/typescript-make-types-global. I have add a comment.

brianmhunt · 2025-02-21T21:55:37Z

global.d.ts

+
+export { };
+
+declare global {


We generally want to be mindful of polluting the global namespace, so we will need to be careful to not distribute this, and not rely on types defined only here.

I agree to 100%. The best global.d.ts is an empty one. Some oldstyle global objects like jquery or required need to be currently declared. The Jasmine and Mocha problem is homemade, but removing Jasmine is a bigger task.

For compatibility, some global knockout types remain here because refactoring leads to invasive changes. Changing the prototype chains of the TKO base classes to js/ts classes can be done in later steps, but it breaks some of the critical interfaces of knockoutjs. We had try that.

I will reorganise some global declarations.

Update: "Import type" was a game changer. I also found a solution to create an object using a function in TypeScript like JavaScript for 'subscribable'. Changing the prototype chain of the objects (classes) in @tko/observable is a different challenge.

brianmhunt · 2025-02-21T21:55:53Z

global.d.ts

+        o: () => ObservableArray // For jsxBehaviors Test
+    }
+
+    //Jasmine and Mocha define duplicated functions, is a problem for the type system


That's how it works for now. Jasmine 1.3 is old and there aren't many tests in Mocha. Perhaps a facade that redirects Jasmine 1.3 to Mocha/Chai is a future solution.

brianmhunt · 2025-02-21T21:57:26Z

packages/bind/spec/asyncBindingBehaviors.ts

@@ -244,14 +244,15 @@ describe('Deferred bindings', function () {

      // Value is initially true, so nodes are retained
    applyBindings({ someItem: someItem, counter: 0 }, testNode)
-    expect(testNode.childNodes[0].childNodes[0].tagName.toLowerCase()).toEqual('span')
+
+    expect(testNode.children[0].children[0].tagName.toLowerCase()).toEqual('span')


unclear why this expectation has changed / needs changing

children is instance of HTMLCollection so tagName is defined on each Element. I will change the other usings of childNodes in this test-case, too.

brianmhunt · 2025-02-21T22:05:03Z

packages/bind/spec/bindingDependencyBehaviors.ts

@@ -449,7 +450,7 @@ describe('Binding dependencies', function () {
      expect(vm.getSubscriptionsCount()).toEqual(1);

            // remove the element and re-set the view-model; the subscription should be cleared
-      testNode.parentNode.removeChild(testNode);
+      testNode.parentNode?.removeChild(testNode);


seems odd that ?. is needed, suggests the test is unstable/flaky (but it may be ok - just noting)

i added expect(testNode.parentElement).not.toBeNull();

(parentNode is optional in HTMLElement)

brianmhunt · 2025-02-21T22:05:58Z

packages/bind/spec/bindingHandlerBehaviors.ts

@@ -53,6 +53,11 @@ describe('BindingHandler behaviors', function () {
      var xCalls = 0,
        yCalls = 0
      bindingHandlers.fnHandler = class extends BindingHandler {
+
+        v : any


just looking at just this file, it's unclear what v, x, y, computed are doing here

Some tests are very interesting.. The member variables are now declared.

brianmhunt · 2025-02-21T22:06:21Z

packages/bind/spec/crossWindowBehaviors.ts

@@ -66,11 +65,11 @@ describe('Cross-window support', function () {

    win2.document.write(BLANK_HTML)
    win2.document.close()
-    body2 = win2.document.body
+    const body2 = win2.document.body


With eslint we can later auto fix all definitions to let or const.. we are all already prepared, see https://github.com/Auge19/tko/tree/add_eslint

brianmhunt · 2025-02-21T22:06:38Z

packages/bind/spec/crossWindowBehaviors.ts


    // renderTemplate
    window.runs(function () {
-      setTemplateEngine(new dummyTemplateEngine({ someTemplate: "<div data-bind='text: text'></div>" }), true)


unclear why we're removing , true (may see it later, just noting)

dummyTemplateEngine and setTemplateEngine have only one parameter, now we a type-safe(r) 👯‍♀️ 👯‍♀️

brianmhunt · 2025-02-21T22:07:21Z

packages/bind/src/DescendantBindingHandler.ts

@@ -2,6 +2,7 @@

 import { applyBindingsToDescendants } from './applyBindings'
 import { AsyncBindingHandler } from './BindingHandler'
+import { BindingResult } from "./BindingResult";


should be import type { BindingResult } ...?

Nice, it could also be a solution for the remaining global KO-defs, so we can move them into the associated modules in a second step. @mcselle FYI "Type-Only Imports": https://www.typescriptlang.org/docs/handbook/modules/reference.html

…emoved

phillipc and others added 30 commits December 20, 2024 21:45

configure typescript

18ea310

add some types

+resolved the frist 1000 warnings

91eb802

#we should remove jasmine..

add some global types

2b994e4

switch d.ts-version for jasmine (the project used jasmine 1.3)

fix: check text/textcontent for comments / add test

2b40fa0

fix some typescript warnings

f105406

fix some types:

11fd46e

1) [] => new Array 2) childNodes => children 3) extend some global type-defs

Merge branch 'dockercontainer_and_fixed_deps_for_testing' into strong…

617c335

…_typed

Readme) some updates for the current stack

4b0b47f

Observable Typings ...

4a25a08

Fixed TSC errors in utils

1081dfb

Utils.Parser fixed tsc error

b4f4e98

Fixed Typo

b47a40a

fix tsc error

8b53a0e

Merge branch 'knockout:main' into strong_typed

d0b3ee6

Merge remote-tracking branch 'origin/fixbrokenTest' into strong_typed

f3abdd4

TS)

4c95467

fix wrong jasmine imports fix method signature

ts) add assertion

5c1be03

Merge branch 'strong_typed' of https://github.com/Auge19/tko into str…

cdcc9e4

…ong_typed

fixing tsc errors

5838247

functionrewrite fix

309b9f9

more tsc fixes

26b3f73

adding props to bindinghandler

9ceaf34

Merge branch 'strong_typed' of https://github.com/Auge19/tko into str…

44b374b

…ong_typed

fixed bindingDependencyBehaviors

a1124a2

utils.component fixes

ed5e0cd

componentBindingBehaviour tsc fixed

0392314

tsc fixes

c9deeff

Merge commit '03923144a2ecb72fc8d41150c1707ae8badf4d24' into strong_t…

1324f71

…yped

foreach tsc fixed

a08a992

slotBinding tsc fixed

b978ee0

mcselle approved these changes Jan 30, 2025

View reviewed changes

Auge19 approved these changes Jan 30, 2025

View reviewed changes

phillipc and others added 10 commits January 30, 2025 15:12

ts) jsx-constructor properties

2127388

Merge branch 'main' into strong_typed

0238738

format) spaces after merging

b7e9f99

add test-case for focus

2e14bc9

add testcase for hasfocus

aee30b1

add expect to test-case

0dd2b7c

debugging) add debugging-configs for vscode

36d3650

fix | HasFocus wont fire in certain scenarios

4251b56

test) fix test-case for electron

d3f04d7

test) remove logs

201947f

phillipc marked this pull request as ready for review February 14, 2025 13:54

fix) options.jquery declaration

5263172

brianmhunt reviewed Feb 21, 2025

View reviewed changes

phillipc added 11 commits February 22, 2025 13:04

ts) reduce some global defs

d74cdb0

ts) some changes after the childNodes/children discussion knockout#202

4f005ab

ts) Type-Only Import

97cd855

ts) use import type / reduce global-def

9960425

ts) reduce global via import type

195c2c7

ts) solution for adding type-defs to subscribable

80881ff

ts) remove global-defs / use any

b7e24df

ts) reduce globals. jQuery isn't overwritten. jQueryInstance can be r…

734627a

…emoved

ts) add a option to generation d.ts-Files (PoC)

49c9f0b

typo

80683fb

ts) fix tsc error in builder-usages

21c7978

phillipc mentioned this pull request Feb 28, 2025

Initial Typescriptification of tko.utils #62

Open

mcselle mentioned this pull request Feb 28, 2025

Fix for hasFocus / checked binding if not applied via DataBindProvider #208

Open

phillipc mentioned this pull request Feb 28, 2025

Update build process #111

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Typescript) fix errors #202

Typescript) fix errors #202

phillipc commented Jan 12, 2025 •

edited

Loading

phillipc commented Feb 14, 2025

brianmhunt left a comment

brianmhunt Feb 21, 2025

phillipc Feb 22, 2025

brianmhunt Feb 21, 2025

brianmhunt Feb 21, 2025

phillipc Feb 22, 2025 •

edited

Loading

brianmhunt Feb 21, 2025

phillipc Feb 22, 2025

phillipc Feb 23, 2025 •

edited

Loading

brianmhunt Feb 21, 2025

phillipc Feb 24, 2025 •

edited

Loading

brianmhunt Feb 21, 2025

phillipc Feb 22, 2025 •

edited

Loading

brianmhunt Feb 21, 2025

phillipc Feb 22, 2025 •

edited

Loading

brianmhunt Feb 21, 2025

phillipc Feb 22, 2025

brianmhunt Feb 21, 2025

phillipc Feb 22, 2025 •

edited

Loading

brianmhunt Feb 21, 2025

phillipc Feb 22, 2025 •

edited

Loading

brianmhunt Feb 21, 2025

phillipc Feb 23, 2025

phillipc Feb 23, 2025


		Note that running `karma` or `rollup` will create a `visual.html` file that shows the proportional size of imports into each package.
		Note that running `karma` will create a `visual.html` file that shows the proportional size of imports into each package.

Typescript) fix errors #202

Are you sure you want to change the base?

Typescript) fix errors #202

Conversation

phillipc commented Jan 12, 2025 • edited Loading

phillipc commented Feb 14, 2025

brianmhunt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phillipc Feb 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phillipc Feb 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phillipc Feb 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phillipc Feb 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phillipc Feb 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phillipc Feb 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phillipc Feb 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phillipc commented Jan 12, 2025 •

edited

Loading

phillipc Feb 22, 2025 •

edited

Loading

phillipc Feb 23, 2025 •

edited

Loading

phillipc Feb 24, 2025 •

edited

Loading

phillipc Feb 22, 2025 •

edited

Loading

phillipc Feb 22, 2025 •

edited

Loading

phillipc Feb 22, 2025 •

edited

Loading

phillipc Feb 22, 2025 •

edited

Loading