Initialize cdk-tree component datasource in the constructor

[jzolnowski]

Due to an error Cannot read properties of undefined (reading 'getTreeData'), had to move datasource initialization to the constructor

[gkalpak]

Where/How did you get the error you mention on the commit message? I don't see how the changes in this PR could make any difference. Can you please give more details on the error and how the changes fix it?

[jzolnowski]

@gkalpak @Splaktar - I managed to analyze the cause of the problem. The root cause of the issue is the tsconfig target ES2022. As it turns out targets ES2021 and ES2022 behave quite differently in some aspects. The following class:

class Data {    
    getTest() {
        return 'test';
    }
}

class Test {
    test1 = this.data.getTest();
    test2 = (() => { console.log("data", this.data)});

    constructor(private data: Data) {
        console.log("getData", this.data.getTest())
    }
}

const test = new Test(new Data())
test.test2();

will be compiled to the form (ES2021):

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
class Data {
    getTest() {
        return 'test';
    }
}
class Test {
    constructor(data) {
        this.data = data;
        this.test1 = this.data.getTest();
        this.test2 = (() => { console.log("data", this.data); });
        console.log("getData", this.data.getTest());
    }
}
const test = new Test(new Data());
test.test2();

/*
Output:
[LOG]: "getData",  "test" 
[LOG]: "data",  Data: {} 
*/

On the other hand, setting the target to ES2022 we get:

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
class Data {
    getTest() {
        return 'test';
    }
}
class Test {
    data;
    test1 = this.data.getTest();
    test2 = (() => { console.log("data", this.data); });
    constructor(data) {
        this.data = data;
        console.log("getData", this.data.getTest());
    }
}
const test = new Test(new Data());
test.test2();

/*
Output:
[ERR]: Cannot read properties of undefined (reading 'getTest') 
*/

As we can see, after compiling, we refer to the class dependency at a time when we do not yet have a class reference created.
Hence, in target ES2022 we get the error

[ERR]: Cannot read properties of undefined (reading 'getTest') 

You can find similar differences in the static` class fields or decorators.

So the solution to our problem is to either initialize the fields of the class in the constructor or restore in tsconfig target to 2021

[gkalpak]

Great sleuthing, @jzolnowski 🕵️

This turns out to be a known issue with TypeScript (microsoft/TypeScript#52331) and it seems there are TS compiler bugs that prevent this from being properly reported as a compile-time error 😞 (microsoft/TypeScript#50971).

This NgPackagr issue also has some useful explanations, work-arounds and references with more details: ng-packagr/ng-packagr#2611

I took a brief look through the code-base and the CdkTreeComponent seems to be the only one that is affected by this.

The fix lgtm 👍

One more thing that needs addressing, though, is that we didn't get a test failure for this.
Is it because we don't cover this part of the app in e2e tests? Could you add a failing test case, @jzolnowski 🙏

[jzolnowski]

One more thing that needs addressing, though, is that we didn't get a test failure for this.

@gkalpak we have this usecase covered

[gkalpak]

Oh, I didn't realize that we don't run tests on CI for this repo 😅
Good to know that the e2e tests were actually failing 👍