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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: super() in class methods is not reported by @babel/eslint-parser #13903

Closed
1 task
ljharb opened this issue Oct 30, 2021 · 11 comments 路 Fixed by #13921
Closed
1 task

[Bug]: super() in class methods is not reported by @babel/eslint-parser #13903

ljharb opened this issue Oct 30, 2021 · 11 comments 路 Fixed by #13921
Labels
area: eslint i: bug outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser

Comments

@ljharb
Copy link
Member

ljharb commented Oct 30, 2021

馃捇

  • Would you like to work on a fix?

How are you using Babel?

@babel/eslint-parser

Input code

        class Foo extends React.Component {
          contructor(props) {
            super(props);
            this.initialValues = {
              test: '',
            };
          }

          render = () => {
            return (
              <Component
                initialValues={this.props.initialValues || this.initialValues}
              >
                {formikProps => (
                  <Input {...formikProps} />
                )}
              </Component>
            );
          }
        }

Configuration file name

No response

Configuration

n/a

Current and expected behavior

I would expect a parse error, because super() is not valid except in constructor() {} and this is contructor() {}.

Environment

v7.16.0 of @babel/eslint-parser, eslint v8.1.10, node v17.0.1

Possible solution

Don't allow super() outside of a class constructor() {}

Additional context

Confusion over this led me to file eslint/eslint#15234, but eslint 8 is actually handling this correctly because of the typo.

related: typescript-eslint/typescript-eslint#4072

@nicolo-ribaudo nicolo-ribaudo changed the title [Bug]: [Bug]: super() in class methods is not reported by @babel/eslint-parser Oct 30, 2021
@JLHwung
Copy link
Contributor

JLHwung commented Oct 30, 2021

Babel with estree plugin already throws: https://astexplorer.net/#/gist/03f767f5dbe210bd68f53a0e841b7d35/453f531ef48973e6054e8ca3860479762c2b626b

and we don't have "contructor" in the codebase anyway, I guess there is nothing can be done on Babel's side?

@ljharb
Copy link
Member Author

ljharb commented Oct 30, 2021

hmm, then why does the test case pass for me in eslint-plugin-react? it fails properly in eslint 8.

@ljharb
Copy link
Member Author

ljharb commented Oct 30, 2021

In AST Explorer, when i select the babel eslint parser, as opposed to the babel parser, it does not throw.

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Nov 1, 2021

I don't know why, but it looks like this is intentional:

However, allowSuperOutsideMethod should probably be changed to behave as if the code was wrapper within class { constructor() { } }.

EDIT: It was introduced in babel/babel-eslint#215

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Nov 1, 2021

We should probably just align with the current ESLint 7 and 8 behavior, since that PR was to align with the old ESLint/espree version.

Or maybe @hzoo you remember more context, even if it was 6 years ago?

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Nov 1, 2021

Other context for a similar behavior: babel/babel-eslint#327.

Since it was enabled on purpose, making it opt-in is a breaking change.

@ljharb
Copy link
Member Author

ljharb commented Nov 1, 2021

oof, ok - so how can i opt out of this behavior?

@nicolo-ribaudo
Copy link
Member

In your eslint config:

parserOpts: {
  babelOptions: {
    allowReturnOutsideFunction: false,
    allowSuperOutsideMethod: false,
  }
}

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Nov 1, 2021

Actually, babel/babel-eslint#327 is opt-in and not by default, so it's different (I can't convince myself if this should be a patch release or a breaking change).

@nicolo-ribaudo
Copy link
Member

Currently ESLint accepts this code (our allowReturnOutsideFunction):

return 2;

@ljharb
Copy link
Member Author

ljharb commented Nov 1, 2021

allowReturnOutsideFunction is correct for CJS and transpiled ESM, but wildly incorrect for native ESM, so it seems like it should depend on the sourceType.

allowSuperOutsideMethod hasn't ever been correct, so i'm baffled why that'd ever be enabled.

@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Feb 4, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: eslint i: bug outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants