no-manage-parent

[ezequiel]

Here's an interesting use case that might be a bit common:

// This component is used in dozens of places in the app
// to easily construct things related to page routes
export function Page({ children, appName }) {

  // ERROR!
  // This effect only uses props. Consider lifting the logic up to the parent. [react-you-might-not-need-an-effect/no-manage-parent]
  useEffect(() => {
    document.title = `My App - ${appName}`;
  }, [appName]);

  return (
    <div>
      {...}
    </div>
  );
};

To avoid duplicating the document.title logic, it's placed in <Page> so it applies to all pages, which seems reasonable

In theory an effect isn't needed for this simple "assignment," but since it's an external side-effect it seems harmless to put it into an effect

Thoughts? Wanted to start a discussion on this simple test case since we'll probably run into more complicated use cases in the future

[NickvanDyke]

Thanks for the discussion! I don't think this should be flagged? And a quick test case confirms that. The plugin recognizes document as an "external" reference, which as you say, is valid.

Make sure your ESLint config has:

import globals from "globals";

languageOptions: {
  globals: {
    ...globals.browser,
  }
}

So that the plugin can resolve document to its globally-defined variable.

The recommended config enables that. If you're using the recommended config but it's still broken, let me know - maybe it doesn't merge with other stuff in the way I thought it does.

[ezequiel]

Ah, got it! We had created a one-off config, so that little bit was missing. I wonder if globals.browser should be auto-baked into the plugin so it just works™️, i.e. globals = {...plugin_assumed_globals, ...user_provided_globals}. I don't feel too strongly either way

import EslintPluginReactYouMightNotNeedAnEffect from 'eslint-plugin-react-you-might-not-need-an-effect';

export default {
  files: ['**/*.{js,jsx,ts,tsx}'],
  plugins: {
    'react-you-might-not-need-an-effect': EslintPluginReactYouMightNotNeedAnEffect,
  },
  rules: {
    'react-you-might-not-need-an-effect/no-manage-parent': 'error',
  },
};

[NickvanDyke]

I wonder if globals.browser should be auto-baked into the plugin so it just works™️

globals is passed to ESLint, not plugin code, and affects the ESLint APIs that the plugin calls. So enabling it in the recommended config is the most I can do unfortunately.

I could add a separate section in the README but 1. it gets pretty noisy because the format is different for the flat and legacy config formats and 2. I expect most people to use the recommended config or already have JSX support and globals.browser enabled by/for other plugins they're using. However I could be wrong about 2.