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

Using exported components in an existing JS/TS React project #4

Closed
muratg opened this issue May 21, 2019 · 25 comments

Comments

Projects
None yet
2 participants
@muratg
Copy link

commented May 21, 2019

Is there a way to make this work? React project build system complains about rules of hooks being broken.

Line 658: React Hook "React.useState" is called in function "exports.useState_" which is neither a React function component or a custom React Hook function react-hooks/rules-of-hooks

This is with a simple component:

mkCounter :: CreateComponent {}
mkCounter = do
  component "Counter" \props -> React.do
    counter /\ setCounter <- useState 0

    -- useEffect counter do
    --   setDocumentTitle $ "Count: " <> show counter
    --   pure mempty

    pure $ fragment
      [ R.h1_ [ R.text $ "New counter!" ]
      , R.label_ [ R.text $ "Hook counter... " <> show counter]
      , R.hr {}
      , R.button
          { onClick: handler_ $ setCounter (_ + 1)
          , children: [ R.text $ "Increment "  ]
          , className: "btn btn-primary"
          }
      , R.button
          { onClick: handler_ $ setCounter (_ - 1)
          , children: [ R.text $ "Decrement" ]
          , className: "btn btn-secondary"
          }
      ]

My goal is to write most of my app with TypeScript, but implement some of the features with PureScript to see if it works for me.

I was able to make react-basic components work in the same TypeScript app, but I'd rather use hooks.

(Thanks for the great lib, BTW!)

@spicydonuts

This comment has been minimized.

Copy link
Owner

commented May 21, 2019

How are you using this component in JS/TS?

@muratg

This comment has been minimized.

Copy link
Author

commented May 21, 2019

  1. Bundle it as a module with spago bundle-module -m Counter -t counter.js
  2. Import it from my TypeScript code with const Counter = require('../purs/counter.js').mkCounter

At this point the watch process errors out with this part of counter.js, specifically React.useState bit:

(function(exports) {
  "use strict";
  var React =require("react");

  exports.useState_ = function(tuple, initialState) {
    var r = React.useState(initialState);
    var state = r[0];
    var setState = r[1];
    return tuple(state, function(update) {
      return function() {
        return setState(update);
      };
    });
  };

  exports.unsafeSetDisplayName = function(displayName, component) {
    component.displayName = displayName;
    component.toString = function() {
      return displayName;
    };
    return component;
  };

  exports.displayName = function(component) {
    return typeof component === "string"
      ? component
      : component.displayName || "[unknown]";
  };
})(PS["React.Basic.Hooks"] = PS["React.Basic.Hooks"] || {});

(I may totally be holding it wrong BTW, but I was able to get a react-basic component work with the same workflow.)

@spicydonuts

This comment has been minimized.

Copy link
Owner

commented May 21, 2019

Can I see the code where you actually import and render mkCounter? It needs to run the CreateComponent effect, i.e.:

import { mkCounter } from '...';

const Counter = mkCounter();

...
  <Counter />
...
@muratg

This comment has been minimized.

Copy link
Author

commented May 21, 2019

This line alone causes it to complain: const Counter = require('../purs/counter.js'). This is without even trying to use the component.

I converted it to use an import statement to see if it works:

import * as counter from '../purs/counter.js'

const Help: React.FC = () => {
  const Counter = counter.mkCounter()
  useDocumentTitle('Help')
  return (
    <div className='Help'>
      <Counter label='test' helpText='what up dude' />
...

... nope. Same error.

Could it be executing the effect while importing/requiring the file?

@muratg

This comment has been minimized.

Copy link
Author

commented May 21, 2019

Note: I created the app with create-react-app --typescript. Perhaps it's an overly aggressive linter rule. 🤔

@spicydonuts

This comment has been minimized.

Copy link
Owner

commented May 21, 2019

Do hooks work in your TS components? It seems like setup somehow but I'm not sure.. can I clone it?

Also, the component creation effect should only happen once at the module level, so you'll want to move that line out of your Help component.

@muratg

This comment has been minimized.

Copy link
Author

commented May 21, 2019

Yeah, I use state and context hooks as well as effects. (Even have a custom hook to set the title.)

This is an internal app, but I'll create a simple project that reproduces the issue after lunch and share it with you.

(Normally that line is at the module level, I was just testing to run it "inside a component" to see if that would change anything.)

@muratg

This comment has been minimized.

Copy link
Author

commented May 21, 2019

Here is a repro app: https://github.com/muratg/hooksrepro

It has three versions of the counter, TypeScript, react-basic, and react-basic-hooks. (The latter is commented out to enable the build.)

@spicydonuts

This comment has been minimized.

Copy link
Owner

commented May 21, 2019

Thanks, I'll take a look soon

@spicydonuts

This comment has been minimized.

Copy link
Owner

commented May 28, 2019

@muratg I'm having trouble understanding how this is set up. I see two PS files, but nothing references them. How are you compiling them?

@muratg

This comment has been minimized.

Copy link
Author

commented May 29, 2019

@spicydonuts the project was created with create-react-app. The PureScript files were included only for your info in that project, they are not used directly. The .js files are.

In case it helps, I shared the project which I used to generate the .js files here: https://github.com/muratg/hooksrepro_libs. You can use the relevant npm script to create either file.

Does this help?

@spicydonuts

This comment has been minimized.

Copy link
Owner

commented May 29, 2019

I'm not quite sure why those bundles aren't working yet, but you could try starting or copying from https://github.com/justinwoo/spacchetti-react-basic-starter/ in the meantime

@muratg

This comment has been minimized.

Copy link
Author

commented May 29, 2019

Thanks! CounterClass.purs in my repro project is similar to what exists in that repo, and it works -- I can use react-basic just fine in my setup, just not react-basic-hooks in the same setup.

My TypeScript project is fully functional components and hooks based (feels much nicer than creating component classes!) So I was hoping to be able to do the same in PureScript parts.

I'll keep looking when I have some free time, perhaps it's something simple that breaks the interaction between the common.js bundle and React rules.

@spicydonuts

This comment has been minimized.

Copy link
Owner

commented May 29, 2019

I thought at first there might be two copies of React bundled, but that doesn't seem to be the case.

@muratg

This comment has been minimized.

Copy link
Author

commented May 29, 2019

I found the problem 🎉 Fixed the symptom in my repro with this commit. See the change in counterHooks.js.

With this change, the error in my first comment above is gone and the component works.

I'm guessing React is having difficulty to identify the custom hook when it's created with exports.useXyz = func... but is fine when it's created with var useXyz = func... and then exported with exports.useXyz = useXyz.

The fix seems simple enough. I'm happy to send a PR if you're interested!

@spicydonuts

This comment has been minimized.

Copy link
Owner

commented May 29, 2019

What do you mean by "identify the custom hook"? It's really strange that this fixes it.

@muratg

This comment has been minimized.

Copy link
Author

commented May 29, 2019

Well, the error in the first place was Line 658: React Hook "React.useState" is called in function "exports.useState_" which is neither a React function component or a custom React Hook function react-hooks/rules-of-hooks.

This suggests that React doesn't think exports.useState_ is a custom hook.

Making the change made this error go away.

@spicydonuts

This comment has been minimized.

Copy link
Owner

commented May 29, 2019

That just means the function was called outside of a React function component. You'd get the same error if you called CounterTS({}) outside of a React component. I think something is finding and calling that function before it should, possibly some create-react-app tooling?

@spicydonuts

This comment has been minimized.

Copy link
Owner

commented May 29, 2019

Oh yes, it's a linting error, not a JS build or runtime error, from the rule react-hooks/rules-of-hook.

@muratg

This comment has been minimized.

Copy link
Author

commented May 29, 2019

I think you're right that create-react-app is involved -- I believe it checks the build output for "rules of hooks".

@spicydonuts

This comment has been minimized.

Copy link
Owner

commented May 29, 2019

Normally the compiled PS is in the folder output, so you could probably add an exception for that folder to skip eslint entirely.

@spicydonuts

This comment has been minimized.

Copy link
Owner

commented May 29, 2019

Ok, going to close this then. Feel free to reopen if I've missed something.

@muratg

This comment has been minimized.

Copy link
Author

commented May 29, 2019

Sure. I'll workaround the issue for now.

@muratg

This comment has been minimized.

Copy link
Author

commented May 29, 2019

One more data point, in case you're interested:

The following also works (Notice how I name the function, instead of keeping it anonymous):

  exports.useState_ = function useState_(tuple, initialState) {
    var r = React.useState(initialState);
    var state = r[0];
    var setState = r[1];
    return tuple(state, function(update) {
      return function() {
        return setState(update);
      };
    });
  };

This shuts up the eslint rule.

Would you consider this change in the library?

@spicydonuts

This comment has been minimized.

Copy link
Owner

commented May 29, 2019

I don't think so. Compiled output isn't supposed to be linted, since it isn't supposed to be edited. This library also has no control over every user's eslint rules, so there isn't even a "correct" version to use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.