docs: new utils commenting their approaches

[Belco90]

No description provided.

[Belco90]

This is the method to find any testing library import from @testing-library/framework or custom module defined in ESLint shared settings. It just needs 3 args:

• context: from the rule, so from there it can get the source code with context.getSourceCode() to find all imports in different formats, and to get context.settings with plugin options.
• utilName: this is the actual Testing Library util you are looking for (e.g. "render", "screen", "waitFor")
• extraModules: an optional one to cover user-event, as it can be imported from custom module or its own package as it doesn't belong to Testing Library frameworks. It could be useful in other cases too.

[Belco90]

Here you can see the first problem. With the current implementation I had, I found this:

Working fine if name imported from testing library

// node
import { screen } from '@testing-library/framework';
// ...

// rule implementation
const importedScreen = getTestingLibraryUtilImport({ context, utilName: 'screen' });
console.log(importedScreen) // 'screen'

Working fine if rename imported from testing library

// node
import { screen as testingScreen } from '@testing-library/framework';
// ...

// rule implementation
const importedScreen = getTestingLibraryUtilImport({ context, utilName: 'screen' });
console.log(importedScreen) // 'testingScreen'

But I don't know what to do here

// node
import * as testingLibrary from '@testing-library/framework';
// ...

// rule implementation
const importedScreen = getTestingLibraryUtilImport({ context, utilName: 'screen' });
console.log(importedScreen) // null

For the 3rd case, this method must return something else to indicate 'screen' has been imported, but its namespaced under testingLibrary object rather than imported directly. How should the method return that then?

[gndelia]

🤔
what about returning an object instead? in addition to the alias (that might be the object exported, an alias, or a full object name that contains all the exported stuff), a enum/flag indicating the type of export so then you know how to treat the string?

[Belco90]

Yeah, I think this is probably the best option. It'll return an object including the imported name and the context of the import. I need to think what to use for the context of the import. I guess I can use same types from AST, so from the example before it would return:

// node
import { screen as testingScreen } from '@testing-library/framework';
// ...

// rule implementation
const importedScreen = getTestingLibraryUtilImport({ context, utilName: 'screen' });
console.log(importedScreen) // { Identifier: 'testingScreen', type: 'ImportSpecifier' }

// node
import * as testingLibrary from '@testing-library/framework';
// ...

// rule implementation
const importedScreen = getTestingLibraryUtilImport({ context, utilName: 'screen' });
console.log(importedScreen) // { Identifier: 'testingLibrary', type: 'ImportNamespaceSpecifier' }

Thoughts?

[Belco90]

At the beginning I was thinking to receive node and settings, but receiving nodes from the rule wouldn't be really useful as the rule would have to look for all different import patterns. Instead, I want this method to receive just context to get both source code and settings. The thing is I've never looked for code/node from source code so it's taking me more time than I thought.

I'm using this eslint-plugin-import rule implementation as reference: /~https://github.com/benmosher/eslint-plugin-import/blob/master/src/rules/no-duplicates.js

[Belco90]

This is a necessary wrapper around getTestingLibraryUtilImport when looking for a render method. Every Testing Library util will be searched just by received utilName but render must be searched also from additional custom names defined in ESLint methods. This method will take care of that by iterating through render and all custom renders and looking for any import matching one of them.

The implementation should work fine already, though it can be improved of course.

[Belco90]

This is how I'm planning to use the new methods, so you can just do this:

create(context) {
  const testingLibraryRenderName = getTestingLibraryRenderImport(context);
  // now you have 'render' within this var if render found and imported from related Testing Library module

  return {
    [`VariableDeclarator CallExpression[callee.name=${testingLibraryRenderName}]`](node) {
      // here you don't need to check anything manually about render alias or imported properly anymore!
      // just implement the actual checks for the rule
    }
  }
}

[gndelia]

I really like this

[Belco90]

Right? I hope I can implement this version, so it would be really easy to focus on specific rules implementations. This is why is taking me so long basically, I'm trying to 1) find proper approach and 2) figure how to implement it. There are couple of things I need to try with context.getSourceCode, I think I might have something.

[Belco90]

Tests updated with the new ESLint shared settings.

[gndelia]

a really minor suggestion, but if we switch from null to use undefined, we can take advantage of find and just do

Suggested change

	let importedRenderName = null;
	possibleRenderOptions.forEach((renderOption) => {
	const importedRenderOption = getTestingLibraryUtilImport({ context, utilName: renderOption});
	if (importedRenderOption) {
	importedRenderName = importedRenderOption
	}
	})
	return importedRenderName
	const importedRenderName = possibleRenderOptions.find((renderOption) => getTestingLibraryUtilImport({ context, utilName: renderOption}));
	return importedRenderName

the only difference is that in case of multiple matches (are we considering that? should we guard or care about that scenario?) find will return the first one but with forEach the last match will be returned

[Belco90]

Ah of course. Much better approach.

About multiple matches: I thought about that possibility. Technically it could happen, but there are so many things I had to consider that I'm gonna just return first match I find. It could happen with something like:

import { render } from '@testing-library/foo';
import { render as customRender } from 'test-utils';

For now we will consider "render" as the single found util method. We can see in the future other alternatives to handle this, but I don't think this is an usual use case.

[Belco90]

There is a case not covered in tests which I forgot to bring here. What about these 2 cases?

    {
      code: `
        import { render } from '@testing-library/foo';

       const setup = () => render(<SomeComponent />);
        
        test('should not report straight destructured render result from custom module', () => {
          const { unmount } = setup();
        });
      `,
      settings: {
        'testing-library/custom-renders': ['setup'],
      },
    },

    {
      code: `
        import { customRender } from 'test-utils';
        
        test('should not report straight destructured render result from custom module', () => {
          const { unmount } = customRender(<SomeComponent />);
        });
      `,
      settings: {
        'testing-library/custom-renders': ['customRender'],
      },
    },

On the first example: "custom-renders" option could be understood as user wants to report setup as render alias, while it's not imported from TL. So getTestingLibraryRenderImport would return just "render" which is the imported render found, but in the rule implementation we would need to still look for those custom renders. I don't think we can automate this somehow with another util to find custom renders not directly imported. I think the most we can do is to add a new util method where you pass the node containing "setup" and found render method returned by getTestingLibraryRenderImport and the method just checks if "setup" (or whatever custom render received) is wrapping given render. This sounds to much for me, I'd leave this up to each rule implementation.

On the second example: "custom-renders" is setup but there is no "custom-modules" setup. Our new util methods won't be aware of this "customRender" as there is no custom module where it could be possibly imported. I think this is fine.