eslint-plugin-jest icon indicating copy to clipboard operation
eslint-plugin-jest copied to clipboard

Snap processor makes enforcing other rules on .snap files impossible

Open scottsheffield opened this issue 5 years ago • 7 comments

Adding the processors to .snap files, and those processors being so explicitly about no-large-snapshots, means I cannot use max-lines, for example, on snapshots to enforce that we are not generating too many snapshots for a given suite.

I appreciate the value of limiting the rules applied to .snap files specifically, but I think it ought to be configurable.

scottsheffield avatar Jul 05 '19 18:07 scottsheffield

I'm afraid I don't understand the issue here

I think it ought to be configurable.

Configurable in what way?

SimenB avatar Jul 19 '19 08:07 SimenB

If I want to run any specific rules on .snap files, right now they're swallowed by the processor because the processor removes anything that isn't no-large-snapshots.

I'd like to see this written more configurably such that the processor could be bypassed / made optional

scottsheffield avatar Aug 07 '19 18:08 scottsheffield

Aha, gotcha. Wanna send a PR? I'm not sure how this is normally handled

SimenB avatar Aug 07 '19 19:08 SimenB

Let me know how you get on w/ this - I have an idea on how to implement a solution, but will leave it for now, as I'm focusing on the TS conversation :)

G-Rath avatar Aug 07 '19 21:08 G-Rath

@scottsheffield we've not had anyone else report this, but it does sound reasonable so if you provide me with an easy reproduction I don't mind looking into it further.

Otherwise I'd like to close this issue off.

G-Rath avatar Jan 01 '22 20:01 G-Rath

I was just about to close this but then had a quick look at our processor and saw:

type PostprocessMessage = { ruleId: string };

export const preprocess = (source: string): string[] => [source];
export const postprocess = (messages: PostprocessMessage[][]) =>
  // snapshot files should only be linted with snapshot specific rules
  messages[0].filter(message => message.ruleId === 'jest/no-large-snapshots');

Unfortunately it looks like there's no way to directly configure processors as they don't get anything from the Context (e.g. so we can't read settings):

    export interface LintMessage {
        /**
         * The 1-based column number.
         */
        column: number;
        /**
         * The 1-based column number of the end location.
         */
        endColumn?: number;
        /**
         * The 1-based line number of the end location.
         */
        endLine?: number;
        /**
         * If `true` then this is a fatal error.
         */
        fatal?: true;
        /**
         * Information for autofix.
         */
        fix?: RuleFix;
        /**
         * The 1-based line number.
         */
        line: number;
        /**
         * The error message.
         */
        message: string;
        messageId?: string;
        nodeType: string;
        /**
         * The ID of the rule which makes this message.
         */
        ruleId: string | null;
        /**
         * The severity of this message.
         */
        severity: Severity;
        source: string | null;
        /**
         * Information for suggestions
         */
        suggestions?: LintSuggestion[];
    }

    export interface Processor {
        /**
         * The function to extract code blocks.
         */
        preprocess?: (text: string, filename: string) => Array<string | {
            text: string;
            filename: string;
        }>;
        /**
         * The function to merge messages.
         */
        postprocess?: (messagesList: Linter.LintMessage[][], filename: string) => Linter.LintMessage[];
        /**
         * If `true` then it means the processor supports autofix.
         */
        supportsAutofix?: boolean;
    }

I would say that the solution is we just shouldn't be filtering out rules and that people should be using overrides to apply the processor + specific rules to .snap files but that would mean more setup for everyone wanting to use no-large-snapshots.

Having said that jest/no-large-snapshots is not a recommended rule so it could be fair to say at least that we shouldn't be enabling the processor as part of recommended maybe?

I'm wondering if we actually even need the processor anymore - without that filter all it does is return the sourcecode as-is, which ESLint might do by default for non-js files (wouldn't be surprised if it didn't though 🤷).

@SimenB what do you think?

G-Rath avatar May 29 '22 20:05 G-Rath

Happy to remove it if it's not needed. Give it a go? 😀

SimenB avatar Aug 28 '22 12:08 SimenB

:tada: This issue has been resolved in version 28.0.0-next.5 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] avatar Mar 27 '24 18:03 github-actions[bot]

:tada: This issue has been resolved in version 28.0.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] avatar Apr 06 '24 08:04 github-actions[bot]