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

[reactive-element] Controller that references a #private field in hostConnected crashes when transpiled using tslib #2446

Open
4 tasks done
bennypowers opened this issue Jan 25, 2022 · 4 comments

Comments

@bennypowers
Copy link
Collaborator

Description

  1. A controller is written in typescript and compiled to use tslib to support ecmascript #private fields.
  2. The controller extends a super class which calls host.addController(this) in its constructor. 3. The controller accessed a private field in its hostConnected callback

When the controller is instantiated on a class field (per typical usage), everything works fine:

import {html, css, LitElement, ReactiveController, ReactiveControllerHost} from 'lit';
import {customElement, property} from 'lit/decorators.js';

class SupCont implements ReactiveController {
  constructor(private host: ReactiveControllerHost) {
    host.addController(this);
  }
  
  hostUpdate() {}
}

class Cont extends SupCont {
  #hasDisconnected = false;
  constructor(
    host: ReactiveControllerHost,
  ) {
    super(host);
  }

  hostConnected(): void {
    if (this.#hasDisconnected) {
      this.#hasDisconnected = false;
    }
  }
}

@customElement('simple-greeting')
export class SimpleGreeting extends LitElement {
  cont = new Cont(this);
  
  render() {
    return html`<p>Hello, World</p>`;
  }
}

lit playground

However, when the controller is instantiated after the constructor, e.g. in connectedCallback, it fails, as tslib's private-field-specific WeakMap is yet uninitialized by the time the controller tries to access it.

Steps to Reproduce

  1. Write this code
import {html, css, LitElement, ReactiveController, ReactiveControllerHost} from 'lit';
import {customElement, property} from 'lit/decorators.js';

class SupCont implements ReactiveController {
  constructor(private host: ReactiveControllerHost) {
    host.addController(this);
  }
  
  hostUpdate() {}
}

class Cont extends SupCont {
  #hasDisconnected = false;
  constructor(
    host: ReactiveControllerHost,
  ) {
    super(host);
  }

  hostConnected(): void {
    if (this.#hasDisconnected) {
      this.#hasDisconnected = false;
    }
  }
}

@customElement('simple-greeting')
export class SimpleGreeting extends LitElement {
  declare cont: Cont;
  
  connectedCallback() {
    super.connectedCallback();
    this.cont = new Cont(this);
  }  
  
  render() {
    return html`<p>Hello, World</p>`;
  }
}
  1. See this output...
Uncaught TypeError: Cannot read private member from an object whose class did not declare it
    __classPrivateFieldGet https://playground.lit.dev/__playground_swfs_f56081d9/2k4abg8/simple-greeting.js:9
    hostConnected https://playground.lit.dev/__playground_swfs_f56081d9/2k4abg8/simple-greeting.js:34
    addController https://playground.lit.dev/__playground_swfs_f56081d9/2k4abg8/node_modules/@lit/reactive-element@1.2.0/development/reactive-element.js:461
    SupCont https://playground.lit.dev/__playground_swfs_f56081d9/2k4abg8/simple-greeting.js:24
    Cont https://playground.lit.dev/__playground_swfs_f56081d9/2k4abg8/simple-greeting.js:30
    connectedCallback https://playground.lit.dev/__playground_swfs_f56081d9/2k4abg8/simple-greeting.js:43
    legacyCustomElement https://playground.lit.dev/__playground_swfs_f56081d9/2k4abg8/node_modules/@lit/reactive-element@1.2.0/development/decorators/custom-element.js:7
    customElement https://playground.lit.dev/__playground_swfs_f56081d9/2k4abg8/node_modules/@lit/reactive-element@1.2.0/development/decorators/custom-element.js:42
    __decorate https://playground.lit.dev/__playground_swfs_f56081d9/2k4abg8/simple-greeting.js:4
    <anonymous> https://playground.lit.dev/__playground_swfs_f56081d9/2k4abg8/simple-greeting.js:49
simple-greeting.js:9:94

Live Reproduction Link

playground link

Expected Results

The controller instantiates without error

Actual Results

Above error thrown

Browsers Affected

  • Chrome
  • Firefox
  • Edge
  • Safari 15
@justinfagnani
Copy link
Member

I don't see any problems in the code there. I think this might be a TypeScript bug.

@jridgewell
Copy link
Contributor

jridgewell commented Jan 25, 2022

This is just an issue with subclass hooks. The subclass's #hasDisconnected field is installed after the call from super() returns. You've called into a subclass hook method (via host.addController(this)'s call to controller.hostConnected()) during the super constructor. So, you've tried to access #hasDisconnected before it's installed.

This is equivalent to the following code:

class Host {
  addController(controller) {
    controller.hostConnected();
  }
}

class BaseController {
  constructor(host) {
    host.addController(this);
  }
}

class SubController extends BaseController {
  constructor(host) {
    super(host);
    /* The field is initialized _after_ super() returns */
    this._hasDisconnected = false;
  }
  
  hostConnected() {
    if (this._hasDisconnected === undefined) {
      throw new Error('Cannot read private member from an object whose class did not declare it');
    }
    if (this._hasDisconnected) {
      this._hasDisconnected = false;
    }
  }
}

new SubController(new Host());
// Error!

@justinfagnani
Copy link
Member

Argh, another problem that Dart's two-pass constructor feature fixes. Thanks for pointing this out @jridgewell

The only robust way I can think of avoiding this is to move the responsibility of adding a controller from the controller itself to the host. That way it can be done after all subclass construction.

export class SimpleGreeting extends LitElement {
  // we'd make addController return the controller
  private cont = this.addController(new Cont(this));
}

or, in connectedCallback:

export class SimpleGreeting extends LitElement {
  private cont?: Cont;
  connectedCallback() {
    super.connectedCallback();
    // we'd make addController return the controller
    this.cont = this.addController(new Cont(this));
  }
}

@justinfagnani justinfagnani changed the title Controller that references a #private field in hostConnected crashes when transpiled using tslib [reactive-element] Controller that references a #private field in hostConnected crashes when transpiled using tslib Jun 18, 2022
@pospi
Copy link

pospi commented Jan 30, 2023

I've encountered this as well, with my project using latest tslib for compilation (2.5.0 at time of writing, with typescript@4.9.4). I see that lit itself is using ^2.0.3.

The relevant line is here, which transpiles to (null===(s=t.hostConnected)||void 0===s||s.call(t)). I suspect that the call syntax breaks the calling context?

However, when the controller is instantiated after the constructor, e.g. in connectedCallback, it fails, as tslib's private-field-specific WeakMap is yet uninitialized by the time the controller tries to access it.

This seems to be what I'm experiencing. For reasons of client connection encapsulation I've been instantiating ApolloQueryController instances inside my component's connectedCallback, which results in this error.

pospi added a commit to neighbour-hoods/timetracking-applet that referenced this issue Jan 30, 2023
GraphQL context now fully resolves async init logic before rendering app elements

adminWebSocket URI removed from GraphQL init to avoid unnecessary Lair auth which Tauri windows are privileged to use without

includes some workarounds to bypass context for GraphQL client, avoiding lit/lit#2446 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 📋 Triaged
Development

No branches or pull requests

4 participants