Fixed decorated methods declared after usage (#813)

[soon]

PR checklist

• Addresses an existing issue: fixes react-bind-this-issue doesn't detect bind decorators defined after usage #813
• New feature, bugfix, or enhancement
  • Includes tests
• Documentation update

Overview of change:

This MR fixes a bug with react-this-binding-issue rule when the decorated method was declared after usage.

[msftclas]

All CLA requirements met.

[JoshuaKGoldberg]

Awesome, LGTM. Thanks so much @soon!

[cyberhck]

thanks a lot :) I couldn't come around to do this, but this will definitely help with all my projects + a few others I know :)

[cyberhck]

any idea when's the new release?

[JoshuaKGoldberg]

@cyberhck good idea! I'll aim for releasing one this weekend.

[cyberhck]

is it working now? For some reason I still don't seem to be correct, I'm using 6.1.1

[soon]

@cyberhck Could you please provide the code which causes the issue?

[cyberhck]

export class HomePage extends React.Component {
  public render(): JSX.Element {
    return (
      <div className={classNames.container}>
        <h2>Register</h2>
        <button onClick={this.handleClick}>Click to register</button>
      </div>
    );
  }

  @autobind
  private async handleClick(): Promise<void> {
    const sdk = Api.getInstance();
    const response = await sdk.users.create({email: "[email protected]", password: "secret", username: "user"});
    console.info("registered new user: ", response);
  }
}

And my tslint has this:

    "react-this-binding-issue": [true, {"bind-decorators": ["autobind"]}],

[soon]

@cyberhck It seems like 6.1.1 does not include these changes. Quick check - download https://registry.npmjs.org/tslint-microsoft-contrib/-/tslint-microsoft-contrib-6.1.1.tgz (or find it in node_modules) and open reactThisBindingIssueRule.js file. It does not have visitClassMember function and node.members.forEach(visitClassMember); call added in tihs PR.

/cc @JoshuaKGoldberg

[cyberhck]

ahh, okay, next release then :) thanks.

[IllusionMH]

Thank you @cyberhck for noticing and @soon for investigation.
There are few PRs that I would like to fit into next release (that will include this fix), however I will release new beta version this weekend even without PRs merged.

[cyberhck]

@IllusionMH hey have you considered using semantic-release for releasing? 🙂 I was wondering if that'd fit this project :)

[IllusionMH]

@cyberhck we've discussed some improvements to release process (and using tools like semantic-release), however no concrete steps yet. Current release workflow is a bit cumbersome and will require effort to migrate (given my limited access to configurations and experience with mentioned tools).

We think of possible improvements to the process again and hopefully come up with more concrete plan. Feel free to create new issue if you have proposals or info to share - would be glad to discuss them.

UPD. I don't think that it will be fully automatic (on CI) as semantic-release supports, but still would make it easier to up version, compose release notes, update docs and publish package and will reduce likelihood of similar issues.

[IllusionMH]

6.2.0-beta was published to npm so you can try it with npm i tslint-microsoft-contrib@beta. 6.2.0 on Wednesday if no blockers.

And sorry for the inconvenience.

P.S. More details on proposal for automation next week

[cyberhck]

And sorry for the inconvenience.

No worries, that's not much of inconvenience actually, being in FOSS world, PR is already merged, so sooner or later we're gonna get it, all we can do is improve the workflow.

P.S. More details on proposal for automation next week

I'm not sure if you saw, but I did create a new issue for this