Code review comments

robatwilliams · Oct 31, 2023 · 6726aa9 · 6726aa9
1 parent 421592b
commit 6726aa9
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 63 deletions.
diff --git a/README.md b/README.md
@@ -57,38 +57,6 @@ It doesn't check browser/runtime-specific APIs (see [eslint-plugin-compat](https
 
 We use a pinned version of `@mdn/browser-compat-data`, because their [SemVer policy](https://github.com/mdn/browser-compat-data#semantic-versioning-policy) allows for breaking changes to the data structure even in minor and patch releases. If you need to use more up to date data, use the `overrides` facility of `package.json` to specify a later version - but be aware that it might break.
 
-## Options
-
-### polyfills
-
-Example:
-
-```
-    'ecmascript-compat/compat': ['error', { polyfills: ['Object.hasOwn'] }]
-```
-
-A array of provided polyfills can be provided in order to not warn about polyfilled features.
-
-### overrideBrowserslist
-
-Example:
-
-```
-    'ecmascript-compat/compat': ['error', { overrideBrowserslist: 'IE >= 11' }]
-```
-
-You can override the browsers list config to a custom definition instead of the default behaviour to find a browsers list file or property in a package.json.
-
-### browserslistOptions
-
-Example:
-
-```
-    'ecmascript-compat/compat': ['error', { browserslistOptions: { env: 'legacy' } }]
-```
-
-You can specify the options provided to browsers list, for example the environment name. See [browserslist API options](https://github.com/browserslist/browserslist#js-api).
-
 ## Limitations
 
 Because JavaScript is untyped, detection of some features' usage (namely prototype methods) through static analysis requires some assumptions to be made. This shouldn't be a problem as long as you avoid creating your own methods having the same names, or write code in an unusual way to deliberately evade detection.

diff --git a/packages/eslint-plugin-ecmascript-compat/README.md b/packages/eslint-plugin-ecmascript-compat/README.md
@@ -27,7 +27,11 @@ npm install --save-dev eslint-plugin-ecmascript-compat
         // Optionally, specify provided polyfills
         "polyfills": [
           "Array.prototype.includes"
-        ]
+        ],
+        // Optionally, override the browsers list config in the eslint config
+        "overrideBrowserslist": "IE >= 11",
+        // Optionally, provide browserslist options - see https://github.com/browserslist/browserslist#js-api
+        "browserslistOptions": { "env": "legacy" }
       }
     ]
   }

diff --git a/packages/eslint-plugin-ecmascript-compat/lib/features/es2022.spec.js b/packages/eslint-plugin-ecmascript-compat/lib/features/es2022.spec.js
@@ -7,84 +7,72 @@ const ruleTester = new RuleTester({
   },
 });
 
-const chrome71Options = { overrideBrowserslist: 'Chrome >= 71' };
-const chrome90Options = { overrideBrowserslist: 'Chrome >= 90' };
+// Browser that doesn't support any features of this version - see es-versions.md
+process.env.BROWSERSLIST = 'Chrome >= 71';
+jest.resetModules();
 
 ruleTester.run('compat', require('../rule'), {
   valid: [
     {
       code: '[].at(1);',
-      options: [
-        { ...chrome71Options, polyfills: ['{Array,String,TypedArray}.prototype.at'] },
-      ],
+      options: [{ polyfills: ['{Array,String,TypedArray}.prototype.at'] }],
     },
     {
       code: '"Foo".at(1);',
-      options: [
-        { ...chrome71Options, polyfills: ['{Array,String,TypedArray}.prototype.at'] },
-      ],
+      options: [{ polyfills: ['{Array,String,TypedArray}.prototype.at'] }],
     },
     {
       code: "new Error('message', { cause: originalError })",
-      options: [{ ...chrome71Options, polyfills: ['Error.cause'] }],
+      options: [{ polyfills: ['Error.cause'] }],
     },
     {
       code: "Object.hasOwn(obj, 'prop');",
-      options: [{ ...chrome71Options, polyfills: ['Object.hasOwn'] }],
+      options: [{ polyfills: ['Object.hasOwn'] }],
     },
   ],
   invalid: [
     {
       // Obvious array, doesn't need aggressive mode
       code: '[].at(1);',
-      options: [chrome71Options],
       errors: [{ message: "ES2022 'Array.prototype.at' method is forbidden." }],
     },
     {
       // Obvious string, doesn't need aggressive mode
       code: '"Foo".at(1);',
-      options: [chrome71Options],
       errors: [{ message: "ES2022 'String.prototype.at' method is forbidden." }],
     },
     {
       // Not obvious, needs aggressive mode
       code: 'foo.at(1);',
-      options: [chrome71Options],
       errors: [{ message: "ES2022 'Array.prototype.at' method is forbidden." }],
     },
     {
       code: 'class A { a = 0 }',
-      options: [chrome71Options],
       errors: [{ message: "ES2020 field 'a' is forbidden." }],
     },
     {
       code: 'class A { static { } }',
-      options: [chrome71Options],
       errors: [{ message: 'ES2022 class static block is forbidden.' }],
     },
     {
       code: "new Error('message', { cause: originalError })",
-      options: [chrome71Options],
       errors: [{ message: 'ES2022 Error Cause is forbidden.' }],
     },
     {
       code: "Object.hasOwn(obj, 'prop');",
-      options: [chrome71Options],
       errors: [{ message: "ES2022 'Object.hasOwn' method is forbidden." }],
     },
     {
       code: '/./d;',
-      options: [chrome71Options],
       errors: [{ message: "ES2022 RegExp 'd' flag is forbidden." }],
     },
     {
       code: 'await true;',
-      options: [chrome71Options],
       errors: [{ message: "ES2022 top-level 'await' is forbidden." }],
     },
     {
       code: 'class A { #field; foo() { #field in this; } }',
-      options: [chrome90Options],
+      options: [{ overrideBrowserslist: 'Chrome >= 90' }],
       errors: [{ message: 'ES2022 private in (`#field in object`) is forbidden.' }],
     },
   ],

diff --git a/packages/eslint-plugin-ecmascript-compat/lib/targetRuntimes.js b/packages/eslint-plugin-ecmascript-compat/lib/targetRuntimes.js
@@ -7,13 +7,6 @@ module.exports = function targetRuntimes(overrideBrowserslist, browserslistOptio
   // ['chrome 50', ...]
   const allNamedVersions = browserslist(overrideBrowserslist, browserslistOptions);
 
-  if (allNamedVersions.length === 0) {
-    // eslint-disable-next-line no-console
-    console.warn(
-      'es-compat: No browserify configurations found, code will not be checked'
-    );
-  }
-
   // [ { name, version }, ... ]
   const all = allNamedVersions.map((namedVersion) => {
     const [name, version] = namedVersion.split(' ');
@@ -35,9 +28,6 @@ module.exports = function targetRuntimes(overrideBrowserslist, browserslistOptio
   const mapped = _.mapKeys(oldestOfEach, (version, name) => mapFamilyName(name));
   const final = _.pickBy(mapped, (version, name) => isKnownFamily(name));
 
-  // eslint-disable-next-line no-console
-  console.log('es-compat: checking compatibility for targets', final);
-
   // [ { name, version } ]
   return Object.entries(final).map(([name, version]) => ({ name, version }));
 };