Merge pull request #219 from Shopify/images-no-direct-imports

Add images-no-direct-imports rule

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 <!-- ## Unreleased -->
 
+### Added
+
+* `images-no-direct-imports` ([#219](https://github.com/Shopify/eslint-plugin-shopify/pull/219))
+
 ## [26.1.2] - 2019-01-02
 
 ### Fixed

README.md

@@ -116,6 +116,7 @@ This plugin provides the following custom rules, which are included as appropria
 
 - [binary-assignment-parens](docs/rules/binary-assignment-parens.md): Require (or disallow) assignments of binary, boolean-producing expressions to be wrapped in parentheses.
 - [class-property-semi](docs/rules/class-property-semi.md): Require (or disallow) semicolons for class properties.
+- [images-no-direct-imports](docs/rules/images-no-direct-imports.md): Prevent images from being directly imported.
 - [jest/no-snapshots](docs/rules/jest/no-snapshots.md): Disallows jest snapshots.
 - [jest/no-vague-titles](docs/rules/jest/no-vague-titles.md): Prevent the usage of vague words in test statements.
 - [jquery-dollar-sign-reference](docs/rules/jquery-dollar-sign-reference.md): Require that all jQuery objects are assigned to references prefixed with `$`.

docs/rules/images-no-direct-imports.md

@@ -0,0 +1,30 @@
+# Prevent directly importing image files and instead force usage of an index file (image-no-direct-imports)
+
+Image files should live in a directory (e.g. `icons`, `illustrations` or `images`) that must contain a dedicated index file that re-exports all the images in that directory.
+
+Files that consume images must import from that index file instead of importing images directly. This rule enforces that convention by disallowing importing directly from an image in any file except the index file in the same dorectory as the image.
+
+## Rule Details
+
+Example of **incorrect** code for this rule:
+
+
+```js
+// components/Foo/Foo.js
+import icon1 from './icons/icon1.svg';
+```
+
+
+Example of **correct** code for this rule:
+
+```js
+// components/Foo/icons/index.js
+export {default as icon1} from './icon1.svg';
+
+// components/Foo/Foo.js
+import {icon1} from './icons';
+```
+
+## When Not To Use It
+
+If you do not wish to enforce import locations for images, then you can safely disable this rule.

index.js

@@ -2,6 +2,7 @@ module.exports = {
   rules: {
     'binary-assignment-parens': require('./lib/rules/binary-assignment-parens'),
     'class-property-semi': require('./lib/rules/class-property-semi'),
+    'images-no-direct-imports': require('./lib/rules/images-no-direct-imports'),
     'jest/no-snapshots': require('./lib/rules/jest/no-snapshots'),
     'jest/no-vague-titles': require('./lib/rules/jest/no-vague-titles'),
     'jquery-dollar-sign-reference': require('./lib/rules/jquery-dollar-sign-reference'),

lib/config/rules/shopify.js

@@ -3,6 +3,8 @@ module.exports = {
   'shopify/binary-assignment-parens': ['error', 'always'],
   // Require (or disallow) semicolons for class properties.
   'shopify/class-property-semi': 'error',
+  // Prevent images from being directly imported
+  'shopify/images-no-direct-imports': 'error',
   // Require that all jQuery objects are assigned to references prefixed with `$`.
   'shopify/jquery-dollar-sign-reference': 'off',
   // Disallow jest snapshots.

lib/rules/images-no-direct-imports.js

@@ -0,0 +1,56 @@
+const {basename, dirname, extname} = require('path');
+const resolve = require('eslint-module-utils/resolve').default;
+const {docsUrl} = require('../utilities');
+
+function isImageImport(resolvedSource) {
+  return /\.(svg|png|jpg)$/.test(resolvedSource);
+}
+
+function isImportFromCurrentFolderIndex(contextFilename, resolvedSource) {
+  const isIndex =
+    basename(contextFilename, extname(contextFilename)) === 'index';
+
+  return isIndex && dirname(resolvedSource) === dirname(contextFilename);
+}
+
+module.exports = {
+  meta: {
+    docs: {
+      description:
+        'Prefer importing image files from the index file of the directory instead of the direct path to the image file.',
+      category: 'Best Practices',
+      recommended: false,
+      uri: docsUrl('images-no-direct-imports'),
+    },
+    fixable: null,
+  },
+  create(context) {
+    function checkNode(node) {
+      if (!node.source || !node.source.value) {
+        return;
+      }
+
+      const resolvedSource = resolve(node.source.value, context);
+
+      if (
+        resolvedSource &&
+        isImageImport(resolvedSource) &&
+        !isImportFromCurrentFolderIndex(context.getFilename(), resolvedSource)
+      ) {
+        context.report({
+          node,
+          message: `Prefer importing image files from the index file of the directory ("{{folderPath}}") instead of the direct path to the image file ("{{filePath}}").`,
+          data: {
+            folderPath: dirname(node.source.value),
+            filePath: node.source.value,
+          },
+        });
+      }
+    }
+    return {
+      ImportDeclaration: checkNode,
+      ExportNamedDeclaration: checkNode,
+      ExportAllDeclaration: checkNode,
+    };
+  },
+};

tests/lib/rules/images-no-direct-imports.js

@@ -0,0 +1,108 @@
+const {RuleTester} = require('eslint');
+const {fixtureFile} = require('../../utilities');
+const rule = require('../../../lib/rules/images-no-direct-imports');
+
+const ruleTester = new RuleTester({
+  parserOptions: {
+    ecmaVersion: 6,
+    sourceType: 'module',
+  },
+});
+
+function errors(type, folderPath, filePath) {
+  return [
+    {
+      type,
+      message: `Prefer importing image files from the index file of the directory ("${folderPath}") instead of the direct path to the image file ("${filePath}").`,
+    },
+  ];
+}
+
+ruleTester.run('images-no-direct-imports', rule, {
+  valid: [
+    // Importing / Exporting svg files from the folder index is valid
+    {
+      code: "import icon1 from './icon1.svg'",
+      filename: fixtureFile('basic-app/app/components/Foo/icons/index.js'),
+    },
+    {
+      code: "import * as icon1 from './icon1.svg'",
+      filename: fixtureFile('basic-app/app/components/Foo/icons/index.js'),
+    },
+    {
+      code: "export {default as icon1} from './icon1.svg'",
+      filename: fixtureFile('basic-app/app/components/Foo/icons/index.js'),
+    },
+    {
+      code: "export * from './icon1.svg'",
+      filename: fixtureFile('basic-app/app/components/Foo/icons/index.js'),
+    },
+    // Importing / Exporting icon index file contents from a component is valid
+    {
+      code: "import {icon1} from './icons'",
+      filename: fixtureFile('basic-app/app/components/Foo/Foo.js'),
+    },
+    {
+      code: "import * as icon1 from './icons'",
+      filename: fixtureFile('basic-app/app/components/Foo/Foo.js'),
+    },
+    {
+      code: "export {default as icon1} from './icons'",
+      filename: fixtureFile('basic-app/app/components/Foo/Foo.js'),
+    },
+    {
+      code: "export * from './icons'",
+      filename: fixtureFile('basic-app/app/components/Foo/Foo.js'),
+    },
+    // Exports without a source file
+    {
+      code: 'export {a, b}; export default c;',
+      filename: fixtureFile('basic-app/app/components/Foo/Foo.js'),
+    },
+  ],
+
+  invalid: [
+    // Importing / Exporting an icon directly from component file is invalid
+    {
+      code: "import icon1 from './icons/icon1.svg'",
+      errors: errors('ImportDeclaration', './icons', './icons/icon1.svg'),
+      filename: fixtureFile('basic-app/app/components/Foo/Foo.js'),
+    },
+    {
+      code: "import * as icon1 from './icons/icon1.svg'",
+      errors: errors('ImportDeclaration', './icons', './icons/icon1.svg'),
+      filename: fixtureFile('basic-app/app/components/Foo/index.js'),
+    },
+    {
+      code: "export {default as icon1} from './icons/icon1.svg'",
+      errors: errors('ExportNamedDeclaration', './icons', './icons/icon1.svg'),
+      filename: fixtureFile('basic-app/app/components/Foo/Foo.js'),
+    },
+    {
+      code: "export * from './icons/icon1.svg'",
+      errors: errors('ExportAllDeclaration', './icons', './icons/icon1.svg'),
+      filename: fixtureFile('basic-app/app/components/Foo/Foo.js'),
+    },
+    // Importing / Exporting an icon directly from some other index file
+    {
+      code: "import icon1 from './icons/icon1.svg'",
+      errors: errors('ImportDeclaration', './icons', './icons/icon1.svg'),
+      filename: fixtureFile('basic-app/app/components/Foo/index.js'),
+    },
+    {
+      code: "import * as icon1 from './icons/icon1.svg'",
+      errors: errors('ImportDeclaration', './icons', './icons/icon1.svg'),
+      filename: fixtureFile('basic-app/app/components/Foo/index.js'),
+    },
+    {
+      code: "export {default as icon1} from './icons/icon1.svg'",
+      errors: errors('ExportNamedDeclaration', './icons', './icons/icon1.svg'),
+      filename: fixtureFile('basic-app/app/components/Foo/index.js'),
+    },
+    {
+      code: "export * from './icons/icon1.svg'",
+      errors: errors('ExportAllDeclaration', './icons', './icons/icon1.svg'),
+      filename: fixtureFile('basic-app/app/components/Foo/index.js'),
+    },
+  ],
+});