Declaring context type on class component breaks static-property-placement

[klimashkin]

Hi,

After upgrading eslint-plugin-react from 7.31.8 to 7.31.9+ (#2581), it started breaking the declare keyword on context in TypeScript:

Not sure what is the mitigation in this case

[ljharb]

The rule has this explicit code:

    // Flow support
    if (node.typeAnnotation && node.key.name === 'context') {
      return true;
    }

We don't seem to have tests for it, though, and I don't see this actual direct code in 54b738d / #1526 - @jomasti, any idea?

[OlivierKamers]

I ran into the same problem and started digging into the source code of lint rules for the first time. I don't understand enough yet to propose a fix but if it helps this could be a test case to reproduce it, which fails if I add it to the valid test cases of this rule:

{
  code: `
    class MyComponent extends React.Component {
      static contextTypes = {
        something: PropTypes.bool
      };
      declare context: MyContext;
    }
  `,
  features: ['ts'],
  options: [STATIC_PUBLIC_FIELD],
},

I will look around a bit more. Even if I don't find a fix it's a good learning experience!

[OlivierKamers]

@ljharb the snippet you mentioned is indeed identifying the context declaration as a contextTypes declaration.
From what I find it doesn't look like it's something that should be supported in Flow either (facebook/flow#7166, facebook/flow#2916) as it's used in conjunction with contextTypes.

Removing this check fixes the failing tests I added and doesn't break any other tests, but as you said they were just missing so there's a risk that it breaks something else.

Another possibility is checking if there is already a sibling that looks like a contextTypes declaration, but it feels weird and not robust.

    // Flow support
    if (node.typeAnnotation && node.key.name === 'context') {
      return !node.parent.body.some((sibling) => isContextTypesDeclaration(sibling));
    }

[ljharb]

Yeah, that doesn't feel like a safe approach. You might be right that it should never have been added, and that it's safe to remove, but I'm hesitant to do that without hearing from @jomasti.

[jomasti]

Sorry, I haven't kept up with notifications at all.

The rule has this explicit code:

    // Flow support
    if (node.typeAnnotation && node.key.name === 'context') {
      return true;
    }

We don't seem to have tests for it, though, and I don't see this actual direct code in 54b738d / #1526 - @jomasti, any idea?

I believe #1533 is the PR instead. That has the change specific to context. It has been quite a while, but I assume I was copying the implementation of isPropTypesDeclaration and updated that inner check without thinking. It most likely can be removed.

[ljharb]

Sounds good, let’s remove it