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

refmterr: Special case brisk-reconciler stateless component error #210

Open
wokalski opened this issue Nov 14, 2019 · 4 comments
Open

refmterr: Special case brisk-reconciler stateless component error #210

wokalski opened this issue Nov 14, 2019 · 4 comments

Comments

@wokalski
Copy link

There's a Revery/brisk-reconciler specific error that's not obvious, it's when you use let%component but don't use hooks inside the component. You get:

This type doesn't match what is expected.

  This type:
    Brisk_reconciler.element(Revery_UI.React.node)

  Expecting:
    Brisk_reconciler__.Hooks.t('a, 'a) =>
    (
      Brisk_reconciler.element('b),
      Brisk_reconciler__.Hooks.t(Brisk_reconciler__.Hooks.nil, 'a)
    )

It'd be great to special case it and explain the root of the cause in human language.

@kyldvs
Copy link
Contributor

kyldvs commented Nov 18, 2019

I wonder if a better solution might be to prevent this error entirely. The ppx could always insert some sort of hook even when none are necessary.

This could always be transformed:

let%component make = () => {
  <View> someContainer </View>
};

Into this:

let%component make = () => {
  let%hook _ = Hooks.empty();
  <View> someContainer </View>
};

Not sure if that will also have some sort of perf impact too though.

@wokalski
Copy link
Author

@kyldvs The PPX as it is now is very simple so you can do things like:

let%component component = (~prop, ()) => implementation(~prop)

Ppx can only be good and reliable if it’s simple. So I’m avoiding any additional magic. There’s enough of it already!

@jordwalke
Copy link
Member

I wonder how we can allow customization on a per library basis. Imagine something like packages being able to append to an environment variable (like a colon separated PATH) except each segment informs refmterr about how to special case errors. What would you want this error to be formatted as?

@wokalski
Copy link
Author

wokalski commented May 20, 2020

@jordwalke Instead of this:

This type doesn't match what is expected.

  This type:
    Brisk_reconciler.element(<ANYTHING>)

  Expecting:
    Brisk_reconciler__.Hooks.t('a, 'a) =>
    (
      Brisk_reconciler.element('b),
      Brisk_reconciler__.Hooks.t(Brisk_reconciler__.Hooks.nil, 'a)
    )

I'd like to match ^ that and output an error saying something along these lines:

let%component can only be used for declaring components that use hooks for state or effects. Remove %component extension if you just return an element from the component.

  This type:
    Brisk_reconciler.element(Revery_UI.React.node)

  Expecting:
    Brisk_reconciler__.Hooks.t('a, 'a) =>
    (
      Brisk_reconciler.element('b),
      Brisk_reconciler__.Hooks.t(Brisk_reconciler__.Hooks.nil, 'a)
    )

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

3 participants