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

Some way for defnc to generate capitalized function names? #100

Closed
aiba opened this issue Jul 28, 2022 · 5 comments
Closed

Some way for defnc to generate capitalized function names? #100

aiba opened this issue Jul 28, 2022 · 5 comments

Comments

@aiba
Copy link
Contributor

aiba commented Jul 28, 2022

Using helix defnc with react-navigation spits out a bunch of warnings because react-navigation requires component functions have a capitalized .name.

This is discussed here:

Weirdly, it seems like React actually specifies that component functions must be named with capital letters. (Unclear if this only applies to JSX or all functional components?)

Can we have a way for defnc to generate function names with capital letters?

Or if I am misinterpreting the React docs, I could file an issue with react-navigation asking them to remove this warning.

@lilactown
Copy link
Owner

The React docs specify that because of the JSX transform logic, not for any internal reason. Take this example:

<foo className="bar baz">qux</div>;

In JSX, it's ambiguous whether the above example should result in creating a React Element using some foo variable (and throw an error if it can't be found), or if it should result in creating a React Element using the string "foo" which the renderer (e.g. react-dom, react-native) knows how to turn into a "native" element.

So the JSX transform has a requirement that all of your components variable names start with UpperCase so that it's unambiguous whether it ought to pass a string to createElement or a variable.

In Helix, we have no such ambiguity because we pass in the exact type we want to create in when creating an element

($ "div") ;; native element
($d "div") ;; native element with DOM prop transformations
($ div) ;; passing in a component bound to the symbol `div`
($d div) ;; passing in a component bound to the symbol `div` with DOM prop transformations

The helix.dom helper macros emit ($d "div") etc.

So we can name our components whatever we want.


For your issue with react-navigation, I would consider opening a ticket with them.

As far as I can tell after skimming the JS source, there's no difference in behavior using a component that starts with an Uppercase vs lowercase other than the warning. I don't know why they took it upon themselves to warn their users, when anyone using JSX should have other ways of being notified about best practices when using that syntax. You can also feel free to ignore the warning.

If the warning is bothering you, you can always name the components you pass to it using an UpperCase name, like

(defnc MyFeature
  [props]
  ,,,)

which should get rid of the warning as well.

@aiba
Copy link
Contributor Author

aiba commented Jul 29, 2022

Thank you for the detailed reply! I will open an issue with react-navigation and reference this one.

Unfortunately(defnc UpperCaseName ..) doesn't even work because the component function's .-name becomes the fully qualified namespace name ("myapp$foo$UpperCaseName_render"), so we would need our root namespace to also be capitalized in order to avoid this warning.

@lilactown
Copy link
Owner

aha right 😅. you could manually set the name property then

(defnc my-component
  [props]
  ,,,)

(set! (.-name my-component) "MyComponent"))

For me, I'd probably just ignore the warning, unless it was really obnoxious.

@aiba
Copy link
Contributor Author

aiba commented Aug 1, 2022

Seems like (on react-native at least), (set! (.-name my-component) "MyComponent")) does not have an impact on (.-name my-component). Perhaps name is some kind of special javascript property?

I was able to get this to use via js/Proxy, but I was worried about what unexpected issues could be caused by swapping a component for a proxied object.

@aiba
Copy link
Contributor Author

aiba commented Aug 1, 2022

For anyone else looking into this issue, I was going to submit an issue to react-navigation, but I failed to reproduce this in a javascript Snack:

https://snack.expo.dev/@aiba/incorrect-warning-repro

I give up.

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

No branches or pull requests

2 participants