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.

Sign up for GitHub

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

Simplify and fix some issues with #[component] macro #2289

Merged
merged 16 commits into from
Aug 8, 2024
Merged

Simplify and fix some issues with #[component] macro #2289

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
550edf7
proper snake case bypass + remove doc building + fix parsing of struc…
Apr 10, 2024
0ff1520
simplify and fix docs
Apr 10, 2024
892d819
Update lib.rs
Apr 10, 2024
3d93646
prefer camel case
Apr 10, 2024
7c56c08
cargo fmt
Apr 10, 2024
b555059
Update lib.rs
Apr 10, 2024
d898b60
fix clippy
Apr 11, 2024
8e5f976
remove lowercase components
Apr 11, 2024
82b1142
Update rsx_usage.rs
Apr 11, 2024
2ac6863
remove `__props` argument in favor of a struct pattern
Apr 11, 2024
53de700
fix clippy
Apr 11, 2024
530a989
remove warning when using camel case
Apr 26, 2024
5fef88e
fix doctests + improve docs
Apr 26, 2024
19e7a3e
revert changes outside of core macro
ealmloff Aug 8, 2024
7ce2bbb
Merge branch 'main' into pr/tigerros/2289
ealmloff Aug 8, 2024
5468018
restore doc changes
ealmloff Aug 8, 2024
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
24 changes: 0 additions & 24 deletions examples/rsx_usage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,6 @@ fn app() -> Element {
Label { text: "hello geneirc world!" }
Label { text: 99.9 }

// Lowercase components work too, as long as they are access using a path
baller::lowercase_component {}

// For in-scope lowercase components, use the `self` keyword
self::lowercase_helper {}

ealmloff marked this conversation as resolved.
Show resolved Hide resolved
// helper functions
// Anything that implements IntoVnode can be dropped directly into Rsx
{helper("hello world!")}
Expand All @@ -229,16 +223,6 @@ fn helper(text: &str) -> Element {
}
}

// no_case_check disables PascalCase checking if you *really* want a snake_case component.
// This will likely be deprecated/removed in a future update that will introduce a more polished linting system,
// something like Clippy.
#[component(no_case_check)]
fn lowercase_helper() -> Element {
rsx! {
"asd"
}
}

mod baller {
use super::*;

Expand All @@ -247,14 +231,6 @@ mod baller {
pub fn Baller() -> Element {
rsx! { "ballin'" }
}

// no_case_check disables PascalCase checking if you *really* want a snake_case component.
// This will likely be deprecated/removed in a future update that will introduce a more polished linting system,
// something like Clippy.
#[component(no_case_check)]
pub fn lowercase_component() -> Element {
rsx! { "look ma, no uppercase" }
}
}

/// Documention for this component is visible within the rsx macro
Expand Down
229 changes: 42 additions & 187 deletions packages/core-macro/src/component.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub struct ComponentBody {
impl Parse for ComponentBody {
fn parse(input: ParseStream) -> Result<Self> {
let item_fn: ItemFn = input.parse()?;
validate_component_fn_signature(&item_fn)?;
validate_component_fn(&item_fn)?;
Ok(Self { item_fn })
}
}
Expand All @@ -22,12 +22,13 @@ impl ToTokens for ComponentBody {
// If there's only one input and the input is `props: Props`, we don't need to generate a props struct
// Just attach the non_snake_case attribute to the function
// eventually we'll dump this metadata into devtooling that lets us find all these components
if self.is_explicit_props_ident() {
//
// Components can also use the struct pattern to "inline" their props.
// Freya uses this a bunch (because it's clean),
// e.g. `fn Navbar(NavbarProps { title }: NavbarProps)` was previously being incorrectly parsed
if self.is_explicit_props_ident() || self.has_struct_parameter_pattern() {
let comp_fn = &self.item_fn;
tokens.append_all(quote! {
#[allow(non_snake_case)]
#comp_fn
});
tokens.append_all(allow_camel_case_for_fn_ident(comp_fn).into_token_stream());
ealmloff marked this conversation as resolved.
Show resolved Hide resolved
return;
}

Expand All @@ -40,7 +41,7 @@ impl ToTokens for ComponentBody {
// No props declared, so we don't need to generate a props struct
true => quote! {},

// Props declared, so we generate a props struct and thatn also attach the doc attributes to it
// Props declared, so we generate a props struct and then also attach the doc attributes to it
false => {
let doc = format!("Properties for the [`{}`] component.", &comp_fn.sig.ident);
let props_struct = self.props_struct();
Expand All @@ -53,8 +54,6 @@ impl ToTokens for ComponentBody {

tokens.append_all(quote! {
#props_struct

#[allow(non_snake_case)]
#comp_fn
});
}
Expand All @@ -75,37 +74,32 @@ impl ComponentBody {
ident: fn_ident,
generics,
output: fn_output,
asyncness,
..
} = sig;

let Generics { where_clause, .. } = generics;
let (_, ty_generics, _) = generics.split_for_impl();
let (_, impl_generics, _) = generics.split_for_impl();

// We generate a struct with the same name as the component but called `Props`
let struct_ident = Ident::new(&format!("{fn_ident}Props"), fn_ident.span());

// We pull in the field names from the original function signature, but need to strip off the mutability
let struct_field_names = inputs.iter().filter_map(rebind_mutability);

let props_docs = self.props_docs(inputs.iter().skip(1).collect());

// Don't generate the props argument if there are no inputs
// This means we need to skip adding the argument to the function signature, and also skip the expanded struct
let props_ident = match inputs.is_empty() {
true => quote! {},
false => quote! { mut __props: #struct_ident #ty_generics },
};
let expanded_struct = match inputs.is_empty() {
true => quote! {},
false => quote! { let #struct_ident { #(#struct_field_names),* } = __props; },
let inlined_props_argument = if inputs.is_empty() {
quote! {}
} else {
quote! { #struct_ident { #(#struct_field_names),* }: #struct_ident #impl_generics }
ealmloff marked this conversation as resolved.
Show resolved Hide resolved
};

// The extra nest is for the snake case warning to kick back in
parse_quote! {
#(#attrs)*
#(#props_docs)*
#asyncness #vis fn #fn_ident #generics (#props_ident) #fn_output #where_clause {
#expanded_struct
#block
#[allow(non_snake_case)]
#vis fn #fn_ident #generics (#inlined_props_argument) #fn_output #where_clause {
{
#block
}
}
}
}
Expand Down Expand Up @@ -139,150 +133,28 @@ impl ComponentBody {
}
}

/// Convert a list of function arguments into a list of doc attributes for the props struct
///
/// This lets us generate set of attributes that we can apply to the props struct to give it a nice docstring.
fn props_docs(&self, inputs: Vec<&FnArg>) -> Vec<Attribute> {
let fn_ident = &self.item_fn.sig.ident;

if inputs.len() <= 1 {
return Vec::new();
}

let arg_docs = inputs
.iter()
.filter_map(|f| build_doc_fields(f))
.collect::<Vec<_>>();

let mut props_docs = Vec::with_capacity(5);
let props_def_link = fn_ident.to_string() + "Props";
let header =
format!("# Props\n*For details, see the [props struct definition]({props_def_link}).*");

props_docs.push(parse_quote! {
#[doc = #header]
});

for arg in arg_docs {
let DocField {
arg_name,
arg_type,
deprecation,
input_arg_doc,
} = arg;

let arg_name = arg_name.into_token_stream().to_string();
let arg_type = crate::utils::format_type_string(arg_type);

let input_arg_doc = keep_up_to_n_consecutive_chars(input_arg_doc.trim(), 2, '\n')
.replace("\n\n", "</p><p>");
let prop_def_link = format!("{props_def_link}::{arg_name}");
let mut arg_doc = format!("- [`{arg_name}`]({prop_def_link}) : `{arg_type}`");

if let Some(deprecation) = deprecation {
arg_doc.push_str("<p>👎 Deprecated");

if let Some(since) = deprecation.since {
arg_doc.push_str(&format!(" since {since}"));
}

if let Some(note) = deprecation.note {
let note = keep_up_to_n_consecutive_chars(&note, 1, '\n').replace('\n', " ");
let note = keep_up_to_n_consecutive_chars(&note, 1, '\t').replace('\t', " ");

arg_doc.push_str(&format!(": {note}"));
}

arg_doc.push_str("</p>");

if !input_arg_doc.is_empty() {
arg_doc.push_str("<hr/>");
}
}

if !input_arg_doc.is_empty() {
arg_doc.push_str(&format!("<p>{input_arg_doc}</p>"));
fn is_explicit_props_ident(&self) -> bool {
if let Some(FnArg::Typed(PatType { pat, .. })) = self.item_fn.sig.inputs.first() {
if let Pat::Ident(ident) = pat.as_ref() {
return ident.ident == "props";
}

props_docs.push(parse_quote! { #[doc = #arg_doc] });
}

props_docs
false
}

fn is_explicit_props_ident(&self) -> bool {
if self.item_fn.sig.inputs.len() == 1 {
if let FnArg::Typed(PatType { pat, .. }) = &self.item_fn.sig.inputs[0] {
if let Pat::Ident(ident) = pat.as_ref() {
return ident.ident == "props";
}
fn has_struct_parameter_pattern(&self) -> bool {
if let Some(FnArg::Typed(PatType { pat, .. })) = self.item_fn.sig.inputs.first() {
if matches!(pat.as_ref(), Pat::Struct(_)) {
return true;
}
}

false
}
}

struct DocField<'a> {
arg_name: &'a Pat,
arg_type: &'a Type,
deprecation: Option<crate::utils::DeprecatedAttribute>,
input_arg_doc: String,
}

fn build_doc_fields(f: &FnArg) -> Option<DocField> {
let FnArg::Typed(pt) = f else { unreachable!() };

let arg_doc = pt
.attrs
.iter()
.filter_map(|attr| {
// TODO: Error reporting
// Check if the path of the attribute is "doc"
if !is_attr_doc(attr) {
return None;
};

let Meta::NameValue(meta_name_value) = &attr.meta else {
return None;
};

let Expr::Lit(doc_lit) = &meta_name_value.value else {
return None;
};

let Lit::Str(doc_lit_str) = &doc_lit.lit else {
return None;
};

Some(doc_lit_str.value())
})
.fold(String::new(), |mut doc, next_doc_line| {
doc.push('\n');
doc.push_str(&next_doc_line);
doc
});

Some(DocField {
arg_name: &pt.pat,
arg_type: &pt.ty,
deprecation: pt.attrs.iter().find_map(|attr| {
if attr.path() != &parse_quote!(deprecated) {
return None;
}

let res = crate::utils::DeprecatedAttribute::from_meta(&attr.meta);

match res {
Err(e) => panic!("{}", e.to_string()),
Ok(v) => Some(v),
}
}),
input_arg_doc: arg_doc,
})
}

fn validate_component_fn_signature(item_fn: &ItemFn) -> Result<()> {
fn validate_component_fn(item_fn: &ItemFn) -> Result<()> {
// Do some validation....
// 1. Ensure the component returns *something*
if item_fn.sig.output == ReturnType::Default {
Expand Down Expand Up @@ -375,38 +247,21 @@ fn rebind_mutability(f: &FnArg) -> Option<TokenStream> {
pat_ident.mutability = None;
}

Some(quote!(mut #pat))
Some(quote!(mut #pat))
}

/// Checks if the attribute is a `#[doc]` attribute.
fn is_attr_doc(attr: &Attribute) -> bool {
attr.path() == &parse_quote!(doc)
}
/// Takes a function and returns a clone of it where an `UpperCamelCase` identifier is allowed by the compiler.
fn allow_camel_case_for_fn_ident(item_fn: &ItemFn) -> ItemFn {
let mut clone = item_fn.clone();
let block = &item_fn.block;

fn keep_up_to_n_consecutive_chars(
input: &str,
n_of_consecutive_chars_allowed: usize,
target_char: char,
) -> String {
let mut output = String::new();
let mut prev_char: Option<char> = None;
let mut consecutive_count = 0;

for c in input.chars() {
match prev_char {
Some(prev) if c == target_char && prev == target_char => {
if consecutive_count < n_of_consecutive_chars_allowed {
output.push(c);
consecutive_count += 1;
}
}
_ => {
output.push(c);
prev_char = Some(c);
consecutive_count = 1;
}
clone.attrs.push(parse_quote! { #[allow(non_snake_case)] });

clone.block = parse_quote! {
{
#block
}
}
};

output
clone
}
Loading
Loading