wix-incubator / vscode-glean

The extension provides refactoring tools for your React codebase
MIT License
1.46k stars 56 forks source link

Error message clarification: 'You must pass a scope and parentPath' #95

Closed DaveClissold closed 3 years ago

DaveClissold commented 4 years ago

Hi I am getting the following error when trying to convert a react PureComponent to stateless.

You must pass a scope and parentPath unless traversing a Program/File. Instead of that you tried to traverse a Identifier node without passing scope and parentPath.

Sorry if this is not the right place, but I am confused and cannot find an answer anywhere.

borislit commented 4 years ago

@DaveClissold Hey! Can you pls post the snippet you are trying to refactor?

Arelav commented 4 years ago

I can't put a production code here however when I try to convert class component to functional it works fine until I put

  "glean.experiments": [
    "hooksForFunctionalComponents"
  ],

in VSCode settings.

borislit commented 4 years ago

@Arelav. Thanks for reporting. Can you try to create a minimal usecase?

Arelav commented 4 years ago

@Arelav. Thanks for reporting. Can you try to create a minimal usecase?

So the minimal example does work without experiment and doesn't when the experiment is on.

import React from 'react';

interface Props {
  text: string;
}

class MyComponent extends React.PureComponent<Props> {
  render() {
    const { text } = this.props;

    return <>Test {text}</>;
  }
}

export default MyComponent;

Looks like props destructuring is an issue here.

Because this works fine in both cases.

class MyComponent extends React.PureComponent<Props> {
  render() {
    return <>Test {this.props.text}</>;
  }
}
bobbyrenwick commented 3 years ago

Seems like there's the same issue whenever you try to spread this.props in JSX too:

class MyComponent extends React.PureComponent<Record<string, unknown>, {}> {
    render() {
        return (
            <OtherComponent {...this.props} />
        );
    }
}

gives the same error

borislit commented 3 years ago

Hey guys! Thanks for reporting! Im on it!

borislit commented 3 years ago

:tada: This issue has been resolved in version 5.1.1 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

borislit commented 3 years ago

Hey guys. Check out the new release. It should fix both the issues you've encountered. Pls confirm. Thanks

bobbyrenwick commented 3 years ago

Hi @borislit, I just updated and it fixed the issue. Thanks :+1: I spend so much time moving from old, class based components to functional ones and this is really helping to speed things up!

borislit commented 3 years ago

@bobbyrenwick Super glad to hear that! If you have suggestion on how we can better the extension in order to make it even more useful to you - I'd love to hear your suggestions :)

omaryoussef commented 3 years ago

Hello! I'm using 5.1.1 and I still see the issue when trying to refactor this:

type Props = { content: Content };

export default class BannerItem extends React.PureComponent<Props> {
  contentDidSelect = () => {
    NavigationService.navigateToContent(this.props.content);
  };

  render() {
    return (
      <TouchableOpacity
        activeOpacity={1}
        style={styles.slideInnerContainer}
        onPress={this.contentDidSelect}>
        <View style={[styles.imageContainer, mainStyles.imageContainerShadow]}>
          <Image
            source={{ uri: this.props.content.image.en }}
            style={styles.image}
          />
        </View>
      </TouchableOpacity>
    );
  }
}
danielo515 commented 3 years ago

I'm also facing this problem, and it's a pity because I have a bunch of components to transform. If you are using regular codemods I may take a look if you link it

borislit commented 3 years ago

Hey @danielo515 , ill have a renewed view on the issue ASAP. I would like to get another snippet that causes the error

borislit commented 3 years ago

@omaryoussef the issue seems to be with function assignment. Once i've removed it - it works. Im working on the fix

borislit commented 3 years ago

:tada: This issue has been resolved in version 5.2.2 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

borislit commented 3 years ago

@omaryoussef @danielo515 I've just released a new version with the fix. Can you please verify?

omaryoussef commented 3 years ago

@borislit Can confirm the issue was fixed, thank you so much! 🎉

danielo515 commented 3 years ago

I confirm this particular bug was fixed on 5.2.2

sirajalam049 commented 3 years ago

I have 5.2.2 installed, still getting the error :(

borislit commented 3 years ago

@sirajalam049 can you pls provide a code snippet?

ghost commented 3 years ago

I am also getting this error with 5.2.2. You can try it with this repo on the App.js file. https://github.com/DevExpress-Examples/DataGrid-How-to-hide-disable-Edit-Form-items-based-on-another-item-s-value