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

Custom Element Scoping broken in 2.3.0 #10016

Open
1 task done
MarcusNotheis opened this issue Oct 14, 2024 · 2 comments
Open
1 task done

Custom Element Scoping broken in 2.3.0 #10016

MarcusNotheis opened this issue Oct 14, 2024 · 2 comments
Assignees
Labels
bug This issue is a bug in the code TOPIC Core

Comments

@MarcusNotheis
Copy link
Collaborator

Bug Description

In versions prior to 2.3.0, the the order of imports and the calling setCustomElementsScopingSuffix did not matter, but starting with 2.3.0, the order is important: Only elements imported after setCustomElementsScopingSuffix was called are scoped now.

This works fine in <2.3.0 and ui5-button-demo gets defined:

import '@ui5/webcomponents/dist/Button.js';
import { setCustomElementsScopingSuffix } from '@ui5/webcomponents-base/dist/CustomElementsScope.js';

setCustomElementsScopingSuffix('demo');

In 2.3.0, the same snippet defines the tag name ui5-button

Example for 2.3.0
Example for 2.2.0

Affected Component

all

Expected Behaviour

The order of imports and the place where setCustomElementsScopingSuffix gets called should not matter.

Isolated Example

https://stackblitz.com/edit/js-djmqeu?file=index.js,index.html

Steps to Reproduce

Open the attached examples from the issue description

Log Output, Stack Trace or Screenshots

No response

Priority

Very High

UI5 Web Components Version

2.3.0

Browser

Chrome, Edge, Firefox, Safari

Operating System

No response

Additional Context

No response

Organization

UI5 Web Components for React / SAP SIX

Declaration

  • I’m not disclosing any internal or sensitive information.
@MarcusNotheis MarcusNotheis added the bug This issue is a bug in the code label Oct 14, 2024
@pskelin pskelin self-assigned this Oct 15, 2024
@pskelin
Copy link
Contributor

pskelin commented Oct 15, 2024

The order of imports and the place where setCustomElementsScopingSuffix gets called should not matter.

It actually does matter and used to work only by chance, because the element's define method is called when an element is imported, but it used to have some async work and by the time the actual customElements.define method was called, the suffix was already known.

This was changed with #9857
Now, when a module for a component is imported, the define is called immediately, hence the suffix should be set before importing any components. Unfortunately, we never documented this expectation, so I'll add it to the docs.

Here is a working example how to do it - you need a separate module that imports the method and sets the suffix, and this module should be imported before any components are imported:
https://stackblitz.com/edit/js-x7vdwq?file=index.js,config-scoping.js

@pskelin
Copy link
Contributor

pskelin commented Oct 15, 2024

a bit of background why this was necessary - frameworks like vuejs or react 19 check whether an element is a component and if a property with this name exists by using 'prop' in el. If this check returns true, a property is set, otherwise an attribute is set.

A normal workflow for apps translates roughly to this:

import '@ui5/webcomponents/dist/CheckBox.js';

// immediately use the component after the import
const cb = document.createElement('ui5-checkbox');
const valueFromState = false;
if ('checked' in cb) {
  // set checked as property
  cb.checked = valueFromState;
} else {
  // set checked as attribute
  cb.setAttribute('checked', valueFromState.toString());
}
document.body.appendChild(cb);

When the value to be set is a boolean false, it was incorrectly set by framworks as a string as the element is not defined and then treated as true by the components later.

Here are two examples how it was broken in 2.2.0
https://stackblitz.com/edit/js-ajqqbc?file=index.html,index.js
and how it works in 2.3.0
https://stackblitz.com/edit/js-azayso?file=index.html,index.js

As you can see, it is important for the define to be synchronous and this leads to the reason for setting the scoping suffix before importing components.

Let us know if you have any ideas how to improve this other than documenting it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug in the code TOPIC Core
Projects
Status: In Progress
Development

No branches or pull requests

3 participants