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

handling attributes in JSX elements #2

Merged
merged 7 commits into from
Oct 17, 2024
Merged

handling attributes in JSX elements #2

merged 7 commits into from
Oct 17, 2024

Conversation

notJoon
Copy link
Collaborator

@notJoon notJoon commented Oct 16, 2024

Description

  • Handling of class Attribute in JSX Elements:

    • The transform_element function now extracts the class attribute from JSX elements and includes it in the generated HTML string.
  • Handling Element without class Attribute:

    • when a class attribute is not exists, the function generates an HTML string that includes only the element tag and its content. (e.g., <div>Hello)

The expected value of the test used the client-side rendering result of SolidJS.

@notJoon notJoon changed the title handling jsx elem with class attribute handling attributes in JSX elements Oct 16, 2024
@notJoon notJoon requested review from XiNiHa and kiwiyou October 16, 2024 07:32
@notJoon notJoon marked this pull request as ready for review October 16, 2024 07:32
crates/core/src/shared/transform.rs Outdated Show resolved Hide resolved
crates/core/src/shared/transform.rs Show resolved Hide resolved
Comment on lines 3 to 4
allocator::{Allocator, Vec as OxcVec},
ast::ast::{self, JSXElementName},
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like these are not used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed 0871357

Comment on lines 250 to 262
let mut child_tmpls = String::new();
let info = TransformInfo::default();
for child in children {
match self.transform_node(child, ctx, &info) {
Some(child_result) => {
if let Some(child_tmpl) = child_result.template {
child_tmpls.push_str(&child_tmpl);
}
}
None => {}
}
}
child_tmpls
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be more performant to use .filter_map() and .collect::<String>() instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was also a clean approach. modified here. 0871357

use super::*;
use oxc::{allocator::Allocator, parser::Parser, semantic::SemanticBuilder, span::SourceType};

struct test_case {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clippy wouldn't like the casing 😅 (I'll set it up in the CI soon)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clippy would love this fix 3c91e3b

@notJoon notJoon requested a review from XiNiHa October 17, 2024 01:14
Copy link
Owner

@XiNiHa XiNiHa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a great starting point!

@XiNiHa XiNiHa merged commit 22cc559 into main Oct 17, 2024
16 of 17 checks passed
@XiNiHa XiNiHa deleted the class-attr branch October 17, 2024 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants