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

Flag to disable upgradable constructor #3202

Open
nolanlawson opened this issue Dec 1, 2022 · 3 comments
Open

Flag to disable upgradable constructor #3202

nolanlawson opened this issue Dec 1, 2022 · 3 comments

Comments

@nolanlawson
Copy link
Collaborator

nolanlawson commented Dec 1, 2022

By default, we define a custom element with an "upgradable constructor":

class UpgradableConstructor extends HTMLElement {
constructor(upgradeCallback: LifecycleCallback) {
super();
// If the element is not created using lwc.createElement(), e.g. `document.createElement('x-foo')`,
// then elementBeingUpgraded will be false
if (elementBeingUpgradedByLWC) {
upgradeCallback(this);
} else if (hasConnectedCallback || hasDisconnectedCallback) {
// If this element has connected or disconnected callbacks, then we need to keep track of
// instances that were created outside LWC (i.e. not created by `lwc.createElement()`).
// If the element has no connected or disconnected callbacks, then we don't need to track this.
elementsUpgradedOutsideLWC.add(this);
// TODO [#2970]: LWC elements cannot be upgraded via new Ctor()
// Do we want to support this? Throw an error? Currently for backwards compat it's a no-op.
}
}
}

This allows us to define multiple LWC components with the same tag name.

However, maybe we should just register one LWC component per tag name. In other words, this would fail:

lwc.createElement('x-foo', { is: A })
lwc.createElement('x-foo', { is: B }) // would throw

Benefits:

To achieve this, we can add a runtime flag (e.g. DISABLE_UPGRADABLE_CONSTRUCTOR).

/cc @caridy

@git2gus
Copy link

git2gus bot commented Dec 1, 2022

This issue has been linked to a new work item: W-12152305

@nolanlawson
Copy link
Collaborator Author

One tricky part will be getting this to work in the Karma tests. A few options:

  1. Launch a new browser window/context for every test (slow, may not work with Jasmine/Karma)
  2. Use a pivot implementation (tricky, might end up testing the polyfill rather than the actual engine code)
  3. Use an iframe (would require creating/destroying an iframe in every test, a bit slower but may be feasible)
  4. Ensure component tag names are unique across tests (lots of refactoring work, but most straightforward approach)

I'm leaning toward #3 or #4. /cc @jmsjtu

@nolanlawson
Copy link
Collaborator Author

If we do this, we will want to remove this line, so that the createCustomElement runs in separate Locker namespaces:

// relies on a shared global cache
createCustomElement,

Incidentally this should be fine per the comment, because the shared global cache would be gone.

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

1 participant