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

[pure-component] Parsing error: Identifier props has already been declared #287

Open
davidnormo opened this issue Dec 1, 2020 · 3 comments

Comments

@davidnormo
Copy link

I'm raising this issue for awareness rather than an ask to fix as I spent some time trying to but found issues in recast which need resolving first.

The Problem

When using the pure-component transform there is an error if a props variable has already been declared in the render method:

// ...inside MyComponent class
render() {
  const { className, ...props } = this.props

The above is transformed into:

function MyComponent(props) {
  const { className, ...props } = props // Parsing error

Investigation

  • I dug in a bit and managed to resolve this trivial case + other variable declaration types (property, rest, member
    expressions) by renaming them to props2
  • However when I looked at declarations in a nested block scope those would also be renamed when ideally they should remain unchanged
    function MyComponent(props) {
      if (true) {
        const props = 'bar' // scoped to the if statement therefore should not be transformed
      }
  • This lead me to look into recast looking up variable bindings by scope and found that recast doesn't support block scoping (path.scope not accurate for const and let benjamn/ast-types#154)
  • If this is sorted out then we can fix this issue more comprehensively in pure-component

Let me know if you want me to share my code so far

@davidnormo
Copy link
Author

davidnormo commented Dec 7, 2020

Update: I've opened a PR here benjamn/ast-types#455

@brayfe
Copy link

brayfe commented Jan 21, 2021

Running into this issue today using the pure-component transform to update React from 15.6 => 16.8. Is the solution here to wait until the above PR is merged? Any guidance would be highly appreciated!

@davidnormo
Copy link
Author

@brayfe the PR is a bit stuck at the moment dealing with hoisting within scopes...

But my changes that fixed my trivial case are here: https://github.com/davidnormo/react-codemod/tree/pure-component-fix
Just beware that it may break some cases if the props variable is inside a block.

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

2 participants