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

Suppression only works for the very first directive (which might not be user-controllable) #240

Open
ghost opened this issue Jun 29, 2023 · 0 comments

Comments

@ghost
Copy link

ghost commented Jun 29, 2023

An attempt was made to workaround bug #206 downstream by using the suppression feature: bvaughn/react-virtualized#1635
However, that workaround didn't seem to work - the problem still exists with the version deployed afterwards.

I didn't try debugging it, but after a quick code-review I theorize this is caused by additional directives being added before babel-plugin-flow-react-proptypes runs. It might be a different reason, but this stands out.


The code only checks the first directive:

const directives = path.node.directives;
if(directives && directives.length) {
const directive = directives[0];
if (directive.value && directive.value.value === SUPPRESS_STRING) {
suppress = true;
}
}

I'm not sure why this isn't something like:

const directives = path.node.directives ?? []; 
directives.forEach((directive) => {
    if (directive.value && directive.value.value === SUPPRESS_STRING) { 
        suppress = true; 
    }
}) 

Obviously a fix for #206 would be more important, but I think the suppression should also be improved.

@ghost ghost changed the title Suppression only works for the very first directive (which might not be use-controllable) Suppression only works for the very first directive (which might not be user-controllable) Jun 29, 2023
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

0 participants