Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-component-cssid.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@lynx-js/web-core": patch
---

Fix componentCSSID behavior for SSR and main thread by calculating element css_id from parent component correctly.
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl MainThreadWasmContext {
parent_component_unique_id: usize,
dom: web_sys::HtmlElement,
dom_ref: js_sys::WeakRef,
css_id: Option<i32>,
component_css_id: Option<i32>,
component_id: Option<String>,
) -> usize {
// unique id
Expand All @@ -89,12 +89,10 @@ impl MainThreadWasmContext {
let unique_id = self.unique_id_to_element_map.len();

let css_id = {
if let Some(css_id) = css_id {
css_id
} else if let Some(parent_component_data) =
if let Some(parent_component_data) =
self.get_element_data_by_unique_id(parent_component_unique_id)
{
parent_component_data.borrow().css_id
parent_component_data.borrow().component_css_id
} else {
0
}
Expand All @@ -106,7 +104,12 @@ impl MainThreadWasmContext {
if css_id != 0 {
let _ = dom.set_attribute(constants::CSS_ID_ATTRIBUTE, &css_id.to_string());
}
let element_data = LynxElementData::new(parent_component_unique_id, css_id, component_id);
let element_data = LynxElementData::new(
parent_component_unique_id,
css_id,
component_css_id.unwrap_or(0),
component_id,
);

let element_data = Box::new(element_data);
self
Expand Down Expand Up @@ -138,14 +141,6 @@ impl MainThreadWasmContext {
None
}

pub fn get_css_id_by_unique_id(&self, unique_id: usize) -> Option<i32> {
self
.unique_id_to_element_map
.get(unique_id)
.and_then(|opt| opt.as_ref())
.map(|element_data_cell| element_data_cell.borrow().css_id)
}

pub fn gc(&mut self) {
let mut ids_to_remove = Vec::new();
for (unique_id, weak_ref) in self.unique_id_to_dom_map.iter() {
Expand Down
11 changes: 10 additions & 1 deletion packages/web-platform/web-core/src/main_thread/element_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pub struct EventHandler {
pub struct LynxElementData {
pub(crate) parent_component_unique_id: usize,
pub(crate) css_id: i32,
pub(crate) component_css_id: i32,
#[with(rkyv::with::Skip)]
pub(crate) component_id: Option<String>,
#[with(rkyv::with::Skip)]
Expand Down Expand Up @@ -54,11 +55,13 @@ impl LynxElementData {
pub(crate) fn new(
parent_component_unique_id: usize,
css_id: i32,
component_css_id: i32,
component_id: Option<String>,
) -> Self {
LynxElementData {
parent_component_unique_id,
css_id,
component_css_id,
component_id,
dataset: None,
component_config: None,
Expand Down Expand Up @@ -152,10 +155,16 @@ impl LynxElementData {
pub(crate) fn new_with_tag_name(
parent_component_unique_id: usize,
css_id: i32,
component_css_id: i32,
component_id: Option<String>,
tag_name: String,
) -> Self {
let mut data = Self::new(parent_component_unique_id, css_id, component_id);
let mut data = Self::new(
parent_component_unique_id,
css_id,
component_css_id,
component_id,
);
data.tag_name = tag_name;
data
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,21 +141,27 @@ impl MainThreadServerContext {
&mut self,
tag_name: String,
parent_component_unique_id: Option<usize>,
css_id_opt: Option<i32>,
component_css_id_opt: Option<i32>,
component_id: Option<String>,
) -> usize {
let id = self.elements.len();
let parent_id = parent_component_unique_id.unwrap_or(0);

let css_id = if let Some(css_id) = css_id_opt {
css_id
} else if let Some(Some(parent_component_data)) = self.elements.get(parent_id) {
parent_component_data.css_id
let css_id = if let Some(Some(parent_component_data)) = self.elements.get(parent_id) {
parent_component_data.component_css_id
} else {
0
};

let mut element = LynxElementData::new_with_tag_name(parent_id, css_id, component_id, tag_name);
let component_css_id = component_css_id_opt.unwrap_or(0);

let mut element = LynxElementData::new_with_tag_name(
parent_id,
css_id,
component_css_id,
component_id,
tag_name,
);
if css_id != 0 {
element.set_attribute(
crate::constants::CSS_ID_ATTRIBUTE.to_string(),
Expand Down Expand Up @@ -571,4 +577,25 @@ mod tests {
// Should not contain the style property since we ignored the empty value
assert!(!html.contains("background-color:"));
}

#[test]
fn test_component_css_id() {
let mut ctx = MainThreadServerContext::new("".to_string(), true, false, false, false);
// Create a component with component_css_id = 42
let comp_id = ctx.create_element(
"x-view".to_string(),
None,
Some(42),
Some("comp1".to_string()),
);

// Create a child element inside this component
// It should inherit css id 42 from the component's component_css_id
let child_id = ctx.create_element("view".to_string(), Some(comp_id), None, None);
ctx.append_child(comp_id, child_id);

let html = ctx.generate_html(comp_id);
// The child should have `l-css-id="42"`
assert!(html.contains("l-css-id=\"42\""), "Generated HTML: {}", html);
}
}
20 changes: 20 additions & 0 deletions packages/web-platform/web-core/tests/element-apis.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,26 @@ describe('Element APIs', () => {
expect(mtsGlobalThis.__GetAttributeByName(ret, 'name')).toBe('name');
});

test('client component_css_id properly cascades to child element', () => {
const root = mtsGlobalThis.__CreatePage('page', 0);
const comp = mtsGlobalThis.__CreateComponent(
0,
'comp1',
42,
'test_entry',
'test_name',
'path',
{},
{},
);
const compId = mtsGlobalThis.__GetElementUniqueID(comp);
const childView = mtsGlobalThis.__CreateElement('view', compId);
mtsGlobalThis.__AppendElement(comp, childView);
mtsGlobalThis.__AppendElement(root, comp);
mtsGlobalThis.__FlushElementTree();
expect(rootDom.querySelector(`[${cssIdAttribute}="42"]`)).not.toBeNull();
});

test('__CreateView', () => {
const ret = mtsGlobalThis.__CreateView(0);
expect(mtsGlobalThis.__GetTag(ret)).toBe('view');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ export function createElementAPI(
__CreateComponent(
parentComponentUniqueId,
componentID,
cssID,
componentCSSID,
entryName,
name,
) {
Expand All @@ -221,7 +221,7 @@ export function createElementAPI(
parentComponentUniqueId,
dom,
new WeakRef(dom),
cssID,
componentCSSID,
componentID,
);
if (entryName) {
Expand Down Expand Up @@ -254,7 +254,7 @@ export function createElementAPI(
);
return dom;
},
__CreatePage(componentID, cssID) {
__CreatePage(componentID, componentCSSID) {
if (page) return page;
const dom = document.createElement(
'div',
Expand All @@ -263,7 +263,7 @@ export function createElementAPI(
0,
dom,
new WeakRef(dom),
cssID,
componentCSSID,
componentID,
);
if (config_default_overflow_visible) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,14 +316,14 @@ export function createElementAPI(
__CreateComponent: ((
parentComponentUniqueId: number,
_componentID: string,
_cssID: number,
componentCSSID: number,
entryName: string,
name: string,
) => {
const id = wasmContext.create_element(
'x-view',
parentComponentUniqueId,
_cssID,
componentCSSID,
_componentID,
); // Component host
const el = { [uniqueIdSymbol]: id } as ServerElement;
Expand Down Expand Up @@ -352,11 +352,11 @@ export function createElementAPI(
);
return { [uniqueIdSymbol]: id } as unknown as DecoratedHTMLElement;
}) as CreateListPAPI,
__CreatePage: ((_componentID: string, _cssID: number) => {
__CreatePage: ((_componentID: string, componentCSSID: number) => {
const id = wasmContext.create_element(
'div',
0,
_cssID,
componentCSSID,
_componentID,
);
pageElementId = id;
Expand Down
7 changes: 5 additions & 2 deletions packages/web-platform/web-core/ts/types/IElementPAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,9 @@ export type UpdateComponentInfoPAPI = (
name?: string;
path?: string;
entry?: string;
/**
* This is component css id but it's named cssID
*/
cssID?: number;
},
) => void;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Expand Down Expand Up @@ -229,7 +232,7 @@ export type CreateWrapperElementPAPI = CreateViewPAPI;
export type CreateComponentPAPI = (
componentParentUniqueID: number,
componentID: string,
cssID: number,
componentCSSID: number,
entryName: string,
name: string,
path: string,
Expand All @@ -245,7 +248,7 @@ export type CreateElementPAPI = (

export type CreatePagePAPI = (
componentID: string,
cssID: number,
componentCSSID: number,
info?: Record<string, Cloneable> | null | undefined,
) => HTMLElement;

Expand Down
Loading