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

React Native support #5

Closed
rp-estebancalvi opened this issue Dec 4, 2019 · 23 comments
Closed

React Native support #5

rp-estebancalvi opened this issue Dec 4, 2019 · 23 comments

Comments

@rp-estebancalvi
Copy link

Hey, guys!
I'm testing the new tool. I wanted to know if you tested it in a React Native project.
I'm trying it on mine, and I could connect the components, but I'm not seeing the code snippet. I'm wondering if that's because of the lack of support.

Cheers and thank you!

@yuqu
Copy link
Member

yuqu commented Dec 5, 2019

Hey @ecalvi,

I'm pretty sure it wouldn't be a problem to generate snippets for React Native components. Did you enable the plugin while connecting?

zeplin connect -p @zeplin/cli-connect-react-plugin

If you did, could you send us a sample config file and a component to investigate what is the problem? Just an example is enough, if you are not able to share for privacy concerns.

@rp-estebancalvi
Copy link
Author

Hey, sure!

This is my config file:

{
  "projects": ["5d0cf133d8b4d25a18dfc854"],
  "styleguides": ["5d0cf5bdbf600559c5b0c9af"],
  "components": [
    {
      "Name": "Button Primary",
      "path": "app/components/ui/button/buttonPrimary.js",
      "zeplinNames": ["Buttons/Primary"],
      "wiki": {
        "urlPath": "https://zeroheight.com/8qj7a0oex/p/82c935/b/54d1b0/t/7955d8"
      }
    },
    {
      "Name": "Button Secondary",
      "path": "app/components/ui/button/buttonSecondary.js",
      "zeplinNames": ["Buttons/Secondary"],
      "wiki": {
        "urlPath": "https://zeroheight.com/8qj7a0oex/p/82c935/b/54d1b0/t/73d26f"
      }
    },
    {
      "Name": "Media Container",
      "path": "app/components/ui/mediaContainer/mediaContainer.js",
      "zeplinNames": ["Media Container"]
    }
  ],
  "github": {
    "repository": "recarga-react",
    "url": "https://github.com/recarga/"
  },
  "links": [
    {
      "name": "Zeroheight",
      "type": "wiki",
      "url": "https://zeroheight.com/8qj7a0oex/p/963a33"
    }
  ]
}

Everything is working wonderfully. I just want to know why isn't there any code snippet or description shown in Zeplin. Is there anything I need to do for it to show? Is there an example react file to see? I looked at the documentation and haven't found any.

Thank you!

@yuqu
Copy link
Member

yuqu commented Dec 6, 2019

It seems there is no problem with the config 🤔 . We're using react-docgen to analyze react components. Could you check out this documentation to find out if your components are constructed like one of the items on the list? You can find the list under Guidelines for default resolvers and handlers section. If it is not one of those, unfortunately we don't support your components for now.

Off topic, I see that you have wiki links on the config. urlPath values of your components are joined with base url defined on links[type=wiki].url. So you should only set urlPath as the remainder of path for each components like below.

  ...
  "components": [
    {
      "Name": "Button Secondary",
      "path": "app/components/ui/button/buttonSecondary.js",
      "zeplinNames": ["Buttons/Secondary"],
      "wiki": {
        "urlPath": "p/82c935/b/54d1b0/t/73d26f"
      }
    },
    ...    // other components
  ],
  ... // other config
  ...
  "links": [
    {
      "name": "Zeroheight",
      "type": "wiki",
      "url": "https://zeroheight.com/8qj7a0oex/"
    }
  ]
}```

@rp-estebancalvi
Copy link
Author

Allright. I hope that's the missing bit. I'll check React-Docgen. Thank you!

@yuqu
Copy link
Member

yuqu commented Dec 16, 2019

Hi @ecalvi, do you have any progress on the issue?

@rp-estebancalvi
Copy link
Author

Sorry, I couldn't make it work last time I tried but haven't had the time to try further. I'm using memo, so maybe that's the problem. Don't know. I think we can close this, though.

It would be nice if you guys could include notes on the proper way to document components if you haven't already.

Thanks!

@yuqu
Copy link
Member

yuqu commented Dec 16, 2019

It is mentioned here that react-docgen was not working with React.memo reactjs/react-docgen#342. They have a fix and it was released 5 days ago. We'll update the library on the next release, until then we can keep this issue open 🤗 .

@yuqu yuqu reopened this Dec 16, 2019
@rp-estebancalvi
Copy link
Author

Oh, nice to hear that! Thank you!

@yuqu
Copy link
Member

yuqu commented Dec 18, 2019

We released an update for @zeplin/cli-connect-react-plugin. Could you try version 0.2.3 and see if that works for your components?

@rp-estebancalvi
Copy link
Author

rp-estebancalvi commented Dec 19, 2019

Sorry, it doesn't seem to be working.

Here's my code:

/**
 * General component description.
 */
function ButtonPrimary(props: Props) {
  return (
    <Button
      block
      disabled={props.loading === true || props.disabled}
      large
      rounded
      onPress={props.onPress}
      testID={generateTestID(props.children)}>
      {props.loading === true ? <LoadingContent {...props} /> : <Text>{props.children}</Text>}
    </Button>
  );
}

export default React.memo<Props>(ButtonPrimary);

I don't know. Could it be that I'm using function MyComponent(props: Props)... instead of class MyComponent extends...?

I'm looking at the changelog here and I couldn't find anything that mentioned support for React.memo.

Thanks for all the help!

@yuqu
Copy link
Member

yuqu commented Dec 20, 2019

React-docgen cannot determine whether the exported item in your example is a React component. Here is a related issue: reactjs/react-docgen#336

The second problem with your example component is that we can only create a useful snippet if react-docgen can give us what kind of properties your component has, it requires an propTypes object literal for that. Even if react-docgen could resolve your component, inputs would still be missing on the code snippet.

We might not support your component for a time but still you can develop a simple plugin like @zeplin/cli-connect-react-plugin that works specifically for your components. We'll publish docs about plugin development very soon 🤗

--

BTW, react-docgen changelog has an item for HOC. The fix was about higher order components in general which cover an issue about React.memo as well. Here is the PR: reactjs/react-docgen#343

@yuqu
Copy link
Member

yuqu commented Dec 20, 2019

I guess I was too quick to jump into a conclusion. On a second look I guess the issue I mentioned may be unrelated, but still the error I got while debugging is that react-docgen cannot find a suitable component. Here you can try it: https://reactcommunity.org/react-docgen/

I see that you use Flow. It actually works fine for Flow react components.

  • When I just remove React.memo it works.
  • When I just remove type info on React.memo like React.memo(ButtonPrimary) it works again.

Not sure what troubles react-docgen's heuristics when there is <Props>, it should not be a problem.

My second note on my last comment is still valid. You would require something like this:

/**
 * General component description.
 */

import * as React from 'react';

type Props = {
  children: any,
  loading: boolean,
  disabled: boolean,
  //  other inputs here
};


function ButtonPrimary(props: Props) {
    return (
      <Button
        block
        disabled={props.loading === true || props.disabled}
        large
        rounded
        onPress={props.onPress}
        testID={generateTestID(props.children)}>
        {props.loading === true ? <LoadingContent {...props} /> : <Text>{props.children}</Text>}
      </Button>
    );
  }

export default React.memo(ButtonPrimary);

@rp-estebancalvi
Copy link
Author

Thank you so much for your help, @yuqu
I'm very limited in terms of what I'm allowed to change when defining components but your suggestions are very valuable to me and I will try it as soon as I get the chance and let you know the results.

Thank you!

@rp-estebancalvi
Copy link
Author

Hello, @yuqu Thanks for the support.
And a happy new year to you.

I'm trying the react-docgen playground you mentioned (thanks for that, btw) with my component as it is and it seems to be working fine, doesn't it?

{
  "description": "General componkjgjhgjhgjhgjhgent description.",
  "displayName": "ButtonPrimary",
  "methods": [],
  "props": {
    "color": {
      "required": true,
      "flowType": {
        "name": "string"
      },
      "description": "Description of prop \"color\"."
    },
    "content": {
      "required": true,
      "flowType": {
        "name": "string"
      },
      "description": "Description of prop \"content\"."
    },
    "children": {
      "required": true,
      "flowType": {
        "name": "any"
      },
      "description": ""
    },
    "disabled": {
      "required": false,
      "flowType": {
        "name": "boolean"
      },
      "description": "",
      "defaultValue": {
        "value": "false",
        "computed": false
      }
    },
    "loading": {
      "required": false,
      "flowType": {
        "name": "boolean"
      },
      "description": ""
    },
    "onPress": {
      "required": false,
      "flowType": {
        "name": "Function"
      },
      "description": ""
    },
    "size": {
      "required": false,
      "flowType": {
        "name": "string"
      },
      "description": ""
    }
  }
}

So I'm guessing the problem is to be in the translation?

@yuqu
Copy link
Member

yuqu commented Jan 2, 2020

Hello @ecalvi 👋 ,

Well, that is very strange 🤔. I use the same component you shared but the output is like the below screenshot. Could it be because you also pasted in the contents of Props type along with the component?

image

@rp-estebancalvi
Copy link
Author

rp-estebancalvi commented Jan 2, 2020

Hello!

Well, ,maybe they updated React-docgen (?). The code I pasted was a simplified version of the one that resulted in the code above, which was the real code.
Anyway, I pasted the simplified version again, the one provided earlier and it also seems to be working, doesn't it?:

{
  "description": "",
  "displayName": "ButtonPrimary",
  "methods": [],
  "props": {
    "children": {
      "required": true,
      "flowType": {
        "name": "any"
      },
      "description": ""
    },
    "loading": {
      "required": true,
      "flowType": {
        "name": "boolean"
      },
      "description": ""
    },
    "disabled": {
      "required": true,
      "flowType": {
        "name": "boolean"
      },
      "description": ""
    }
  }
}

Where could the problem be? Any ideas?

@rp-estebancalvi
Copy link
Author

rp-estebancalvi commented Jan 6, 2020

Hey, guys!
You know it would be nice to have some kind of logging.

I'm syncing components and everything seems to work:

Zeplin CLI - v0.2.7

Connecting all connected components into Zeplin…
🦄 Components successfully connected to components in Zeplin

But I'm not seeing the code snippets in Zeplin even though everything seems to be working fine and the code is working in the react-gen playground. It seems something is getting lost in the translation but since there is no logging I'm not able to know. :-(

Cheers!

@yuqu
Copy link
Member

yuqu commented Jan 7, 2020

Hey @ecalvi, sorry for the late response. I'm still not sure about why would you can see the output on react docgen playground but I could not. I would expect to see the results as you have.

About logging, we're working on a log file output which you will be able to see the output for each component and any other process happening. I'll notify you when is it ready.

In the meantime, could you share the output of the following commands?
npm info @zeplin/cli
npm info @zeplin/cli-connect-react-plugin

Also could you share the full command you are using to connect?

@rp-estebancalvi
Copy link
Author

rp-estebancalvi commented Jan 7, 2020

Hello, @yuqu !
No problem. Thank you for your response and sorry for the length of mine. :-p

Regarding the playground, I tried again my code in an incognito window:

// @flow

import { Button, Text } from 'native-base';

import { ActivityIndicator } from 'react-native';
import React from 'react';
// Prevent Objects to breaking when used as value on testID
import { generateTestID } from '../helpers';
import variables from '../../../styles/variables';

type Props = {
  /**
   * Description of prop "color".
   */
  color: string,
  content: string,
  children: any,
  disabled?: boolean,
  loading?: boolean,
  onPress?: Function,
  size?: string
};

LoadingContent.defaultProps = {
  content: 'Processando'
};

ButtonPrimary.defaultProps = {
  disabled: false
};

function LoadingContent(props: Props) {
  return (
    <>
      <Text>{props.content}</Text>
      <ActivityIndicator size={'small'} color={variables.inverseSpinnerColor} />
    </>
  );
}

/**
 * General component description.
 */
function ButtonPrimary(props: Props) {
  return (
    <Button
      block
      disabled={props.loading === true || props.disabled}
      large
      rounded
      onPress={props.onPress}
      testID={generateTestID(props.children)}>
      {props.loading === true ? <LoadingContent {...props} /> : <Text>{props.children}</Text>}
    </Button>
  );
}

export default React.memo<Props>(ButtonPrimary);

And yes, it seems to be working:

{
  "description": "General component description.",
  "displayName": "ButtonPrimary",
  "methods": [],
  "props": {
    "color": {
      "required": true,
      "flowType": {
        "name": "string"
      },
      "description": "Description of prop \"color\"."
    },
    "content": {
      "required": true,
      "flowType": {
        "name": "string"
      },
      "description": ""
    },
    "children": {
      "required": true,
      "flowType": {
        "name": "any"
      },
      "description": ""
    },
    "disabled": {
      "required": false,
      "flowType": {
        "name": "boolean"
      },
      "description": "",
      "defaultValue": {
        "value": "false",
        "computed": false
      }
    },
    "loading": {
      "required": false,
      "flowType": {
        "name": "boolean"
      },
      "description": ""
    },
    "onPress": {
      "required": false,
      "flowType": {
        "name": "Function"
      },
      "description": ""
    },
    "size": {
      "required": false,
      "flowType": {
        "name": "string"
      },
      "description": ""
    }
  }
}

Regarding the command, this is the command I'm using:

{
  "scripts": {
    "library-sync": "zeplin connect"
  }
}

About those commands you ask, this is their output:

@zeplin/cli@0.3.1 | MIT | deps: 13 | versions: 16
Zeplin CLI
https://github.com/zeplin/cli#readme

bin: zeplin

dist
.tarball: https://registry.npmjs.org/@zeplin/cli/-/cli-0.3.1.tgz
.shasum: e448e3a52680b88ae569d099dcff317c60b4c002
.integrity: sha512-7l0H1TjigJzHTg2TRZdw59rwLkcgWljQ520RYgdjgzKHv7qswux4O7N8QGLs6DV+Q2PUgynNG1O2zhkzJMXH4w==
.unpackedSize: 207.5 kB

dependencies:
@hapi/joi: ^16.1.7        ci-info: ^2.0.0           express: ^4.17.1          inquirer: ^7.0.0          url-join: ^4.0.1          
axios: ^0.19.0            commander: ^3.0.1         fs-extra: ^8.1.0          jsonwebtoken: ^8.5.1      
chalk: ^3.0.0             endent: ^1.3.0            http-status-codes: ^1.4.0 ts-dedent: ^1.1.0         

maintainers:
- ceylanismail <ismail@zeplin.io>
- dirtybit <sertac@zeplin.io>
- mammet <muhammet@zeplin.io>
- merih <merihakar@gmail.com>
- zeplin-npm <npm@zeplin.io>

dist-tags:
latest: 0.3.1  

published a week ago by zeplin-npm <npm@zeplin.io>

&

@zeplin/cli-connect-react-plugin@0.2.3 | MIT | deps: 3 | versions: 5
Zeplin CLI Connected Components - React Plugin
https://github.com/zeplin/cli-connect-react-plugin#readme

dist
.tarball: https://registry.npmjs.org/@zeplin/cli-connect-react-plugin/-/cli-connect-react-plugin-0.2.3.tgz
.shasum: 1d4218b5dbc7f033c49160507094d7632c2c9809
.integrity: sha512-rVfcge16WhR2FQrhXhO/KJCIeEA3wSSnCw4N9NMA6RU/bZTBIaMHwGt+MIZZ72ZkD/2sDJin7EGJDFbJyfKoeQ==
.unpackedSize: 12.2 kB

dependencies:
fs-extra: ^8.1.0     pug: ^2.0.4          react-docgen: ^5.0.0 

maintainers:
- ceylanismail <ismail@zeplin.io>
- dirtybit <sertac@zeplin.io>
- mammet <muhammet@zeplin.io>
- merih <merihakar@gmail.com>
- zeplin-npm <npm@zeplin.io>

dist-tags:
latest: 0.2.3  

published 2 weeks ago by zeplin-npm <npm@zeplin.io>

I also tried updating react-docgen only to no avail.

Thank you!

@yuqu
Copy link
Member

yuqu commented Jan 7, 2020

I think I found what is missing. The script command is missing plugin declaration. Update it like below and give it a try 🤞 Since plugins npm package names can be anything, they are required to be declared when running connect command

{
  "scripts": {
    "library-sync": "zeplin connect -p @zeplin/cli-connect-react-plugin"
  }
}

@rp-estebancalvi
Copy link
Author

It worked! Thanks, @yuqu

Thanks so much for your help.
I'll be coming back with further issues in the future. :-)

@jackkoppa
Copy link

great find, @yuqu, this has been quite a ride to follow. Glad it's settled!

@rp-estebancalvi
Copy link
Author

Thanks, everyone!

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