dts-critic - npm Package Compare versions

Comparing version 1.0.3 to 1.0.4

dt.js

@@ -6,3 +6,3 @@ const critic = require('./index')
 /** @param {string} tslintPath */
-async function hasDtHeaderLintRule(tslintPath) {
+function hasDtHeaderLintRule(tslintPath) {
     if (fs.existsSync(tslintPath)) {
@@ -18,9 +18,8 @@ const tslint = JSON.parse(stripJsonComments(fs.readFileSync(tslintPath, 'utf-8')))
 
-
-async function main() {
+function main() {
     for (const item of fs.readdirSync('../DefinitelyTyped/types')) {
         const entry = '../DefinitelyTyped/types/' + item
         try {
-            if (await hasDtHeaderLintRule(entry + '/tslint.json')) {
-                await critic(entry + '/index.d.ts')
+            if (hasDtHeaderLintRule(entry + '/tslint.json')) {
+                critic(entry + '/index.d.ts')
             }
@@ -37,6 +36,17 @@ }
                 console.log('trying to add ' + e.homepage + '...')
-                // const re = /\/\/ Project: (.+)/;
-                // const s = fs.readFileSync(entry + '/index.d.ts', 'utf-8')
-                // fs.writeFileSync(entry + '/index.d.ts', s.replace(re, '// Project: $1, ' + e.homepage), 'utf-8')
+                const re = /\/\/ Project: (.+)/;
+                const s = fs.readFileSync(entry + '/index.d.ts', 'utf-8')
+                fs.writeFileSync(entry + '/index.d.ts', s.replace(re, '// Project: $1, ' + e.homepage), 'utf-8')
             }
+            else if (/but the source does not mention 'default'/.test(e.message)) {
+                console.log('converting', item, 'to export = ...')
+                const named = /export default function\s+(\w+\s*)\(/;
+                const anon = /export default function\s*\(/;
+                const id = /export default(\s+\w+);/;
+                let s = fs.readFileSync(entry + '/index.d.ts', 'utf-8')
+                s = s.replace(named, 'export = $1;\ndeclare function $1(')
+                s = s.replace(anon, 'export = _default;\ndeclare function _default(')
+                s = s.replace(id, 'export =$1;')
+                fs.writeFileSync(entry + '/index.d.ts', s, 'utf-8')
+            }
             else {
@@ -43,0 +53,0 @@ console.log('*** ERROR for ' + item + ' ***')

index.d.ts

@@ -1,2 +0,2 @@
-declare function dtsCritic(dtsPath: string, sourcePath?: string): Promise<void>;
+declare function dtsCritic(dtsPath: string, sourcePath?: string): void;
 export = dtsCritic;

index.js

@@ -5,3 +5,3 @@ const yargs = require("yargs");
 const path = require("path");
-const request = require("request-promise-native");
+const download = require("download-file-sync");
 
@@ -12,3 +12,3 @@ /**
  */
-async function dtsCritic(dtsPath, sourcePath) {
+function dtsCritic(dtsPath, sourcePath) {
     const dts = fs.readFileSync(dtsPath, "utf-8");
@@ -22,3 +22,10 @@ let header;
     }
-    check(await findNames(dtsPath, sourcePath, header), header);
+    const names = findNames(dtsPath, sourcePath, header)
+    const src = sourcePath ?
+        fs.readFileSync(require.resolve(sourcePath), "utf-8") :
+        download("https://unpkg.com/" + mangleScoped(names.src));
+    checkNames(names, header);
+    if (header && !header.nonNpm) {
+        checkSource(names.dts, dts, src);
+    }
 }
@@ -28,3 +35,4 @@ dtsCritic.findDtsName = findDtsName;
 dtsCritic.retrieveNpmHomepageOrFail = retrieveNpmHomepageOrFail;
-dtsCritic.check = check;
+dtsCritic.checkNames = checkNames;
+dtsCritic.checkSource = checkSource;
 
@@ -34,3 +42,3 @@ module.exports = dtsCritic;
 if (!module.parent) {
-    main().catch(e => { console.log(e); process.exit(1) });
+    main();
 }
@@ -46,3 +54,3 @@
 
-async function main() {
+function main() {
     const argv = yargs.
@@ -53,23 +61,16 @@ usage("$0 name.d.ts [source-folder]\n\nIf source-folder is not provided, I will look for a matching package on npm.", ).
     if (argv._.length === 0) {
-        console.log('Please provide a path to a d.ts file for me to critique.');
+        console.log("Please provide a path to a d.ts file for me to critique.");
         process.exit(1);
     }
-    let header;
-    const dts = fs.readFileSync(argv._[0], "utf-8");
-    try {
-        header = headerParser.parseHeaderOrFail(dts);
-    }
-    catch(e) {
-        header = undefined;
-    }
-    check(await findNames(argv._[0], argv._[1], header), header);
+    return dtsCritic(argv._[0], argv._[1]);
 }
 
 /**
+ * Find package names of dts and source. Also finds the homepage from the DT header, if present.
  * @param {string} dtsPath
  * @param {string | undefined} sourcePath
  * @param {headerParser.Header | undefined} header
- * @return {Promise<Names>}
+ * @return {Names}
  */
-async function findNames(dtsPath, sourcePath, header) {
+function findNames(dtsPath, sourcePath, header) {
     const dts = findDtsName(dtsPath);
@@ -85,3 +86,3 @@ let src;
         try {
-            homepage = await retrieveNpmHomepageOrFail(dts);
+            homepage = retrieveNpmHomepageOrFail(dts);
             nonNpmHasMatchingPackage = !!header && header.nonNpm && !isExistingSquatter(dts);
@@ -119,12 +120,20 @@ }
 
-/** @param {string} dts */
-function isExistingSquatter(dts) {
-    return dts === "atom" ||
-        dts === "ember__string" ||
-        dts === "fancybox" ||
-        dts === "jsqrcode" ||
-        dts === "node" ||
-        dts === "titanium";
+/** @param {string} name */
+function isExistingSquatter(name) {
+    return name === "atom" ||
+        name === "ember__string" ||
+        name === "fancybox" ||
+        name === "jsqrcode" ||
+        name === "node" ||
+        name === "geojson" ||
+        name === "titanium";
 }
 
+/** @param {string} name */
+function isRealExportDefault(name) {
+    return name.indexOf("react-native") > -1 ||
+        name === "ember-feature-flags" ||
+        name === "material-ui-datatables";
+}
+
 /**
@@ -153,6 +162,10 @@ * If dtsName is 'index' (as with DT) then look to the parent directory for the name.
  * @param {string} baseName
- * @return {Promise<string>}
+ * @return {string}
  */
-async function retrieveNpmHomepageOrFail(baseName) {
-    return JSON.parse(await request("https://registry.npmjs.org/" + mangleScoped(baseName))).homepage;
+function retrieveNpmHomepageOrFail(baseName) {
+    const npm = JSON.parse(download("https://registry.npmjs.org/" + mangleScoped(baseName)));
+    if ("error" in npm) {
+        throw new Error(baseName + " " + npm.error);
+    }
+    return npm.homepage;
 }
@@ -172,3 +185,3 @@
  */
-function check(names, header) {
+function checkNames(names, header) {
     if (names.dts !== names.src) {
@@ -193,2 +206,20 @@ throw new Error(`d.ts name '${names.dts}' must match source name '${names.src}'.`);
 
+/**
+ * A d.ts with 'export default' and no ambient modules should have source that contains
+ * either 'default' or '__esModule' or 'react-side-effect' or '@flow' somewhere.
+ * This function also skips any package named 'react-native'.
+ *
+ * Note that this function doesn't follow requires, but just tries to detect
+ * 'module.exports = require'
+ * @param {string} name
+ * @param {string} dts
+ * @param {string} src
+ */
+function checkSource(name, dts, src) {
+    if (dts.indexOf("export default") > -1 && !/declare module ['"]/.test(dts) &&
+        !isRealExportDefault(name) && src.indexOf("default") === -1 && src.indexOf("__esModule") === -1 && src.indexOf("react-side-effect") === -1 && src.indexOf("@flow") === -1 && src.indexOf("module.exports = require") === -1) {
+        throw new Error(`The types for ${name} specify 'export default' but the source does not mention 'default' anywhere.`);
+    }
+}
+
 /** @param {string} url */
@@ -195,0 +226,0 @@ function normalise(url) {

index.test.js

@@ -1,2 +0,2 @@
-const { findDtsName, findNames, retrieveNpmHomepageOrFail, check } = require("./index");
+const { findDtsName, findNames, retrieveNpmHomepageOrFail, checkNames, checkSource } = require("./index");
 /**
@@ -31,4 +31,4 @@ * @param {string} description
 suite("findNames", {
-    async absolutePathsBoth() {
-        expect(await findNames("jquery/index.d.ts", "~/dts-critic", undefined)).toEqual({
+    absolutePathsBoth() {
+        expect(findNames("jquery/index.d.ts", "~/dts-critic", undefined)).toEqual({
             dts: "jquery",
@@ -40,4 +40,4 @@ src: "dts-critic",
     },
-    async currentDirectorySource() {
-        expect(await findNames("jquery/index.d.ts", ".", undefined)).toEqual({
+    currentDirectorySource() {
+        expect(findNames("jquery/index.d.ts", ".", undefined)).toEqual({
             dts: "jquery",
@@ -49,4 +49,4 @@ src: "dts-critic",
     },
-    async mistakenFileNameSource() {
-        expect(await findNames("jquery/index.d.ts", "/home/lol/oops.index.js", undefined)).toEqual({
+    mistakenFileNameSource() {
+        expect(findNames("jquery/index.d.ts", "/home/lol/oops.index.js", undefined)).toEqual({
             dts: "jquery",
@@ -58,4 +58,4 @@ src: "lol",
     },
-    async trailingSlashSource() {
-        expect(await findNames("jquery/index.d.ts", "/home/lol/", undefined)).toEqual({
+    trailingSlashSource() {
+        expect(findNames("jquery/index.d.ts", "/home/lol/", undefined)).toEqual({
             dts: "jquery",
@@ -67,13 +67,11 @@ src: "lol",
     },
-    async mismatchPackageFailNoHeader() {
+    mismatchPackageFailNoHeader() {
         // surely parseltongue will never exist
-        expect.assertions(1)
-        try {
-            await findNames("parseltongue.d.ts", undefined, undefined)
-        }
-        catch (e) {
-            expect(e.message).toEqual(`d.ts file must have a matching npm package.
+        expect(() => findNames("parseltongue.d.ts", undefined, undefined)).toThrow(
+            `d.ts file must have a matching npm package.
 To resolve this error, either:
-Add a Definitely Typed header with the first line
+1. Change the name to match an npm package.
+2. Add a Definitely Typed header with the first line
 
+
     // Type definitions for non-npm package parseltongue-browser
@@ -85,10 +83,8 @@
 
-Explicitly provide dts-critic with a source file. This is not allowed for submission to Definitely Typed.`)
-        }
+3. Explicitly provide dts-critic with a source file. This is not allowed for submission to Definitely Typed.
+`)
     },
-    async mismatchPackageFailNpmHeader() {
+    mismatchPackageFailNpmHeader() {
         // surely parseltongue will never exist
-        expect.assertions(1)
-        try {
-            await findNames("parseltongue.d.ts", undefined, {
+        expect(() => findNames("parseltongue.d.ts", undefined, {
                 nonNpm: false,
@@ -101,9 +97,9 @@ libraryName: "a",
                 projects: ["welcome-to-zombo.com", "this-is-zombo.com"]
-            })
-        }
-        catch (e) {
-            expect(e.message).toEqual(`d.ts file must have a matching npm package.
+        })).toThrow(
+            `d.ts file must have a matching npm package.
 To resolve this error, either:
-Add a Definitely Typed header with the first line
+1. Change the name to match an npm package.
+2. Add a Definitely Typed header with the first line
 
+
     // Type definitions for non-npm package parseltongue-browser
@@ -115,10 +111,8 @@
 
-Explicitly provide dts-critic with a source file. This is not allowed for submission to Definitely Typed.`)
-        }
+3. Explicitly provide dts-critic with a source file. This is not allowed for submission to Definitely Typed.
+`)
     },
-    async mismatchPackageFailNonNpmHeader() {
+    mismatchPackageFailNonNpmHeader() {
         // surely parseltongue will never exist
-        expect.assertions(1)
-        try {
-            await findNames("jquery.d.ts", undefined, {
+        expect(() => findNames("jquery.d.ts", undefined, {
                 nonNpm: true,
@@ -131,36 +125,28 @@ libraryName: "a",
                 projects: ["welcome-to-zombo.com", "this-is-zombo.com"]
-            })
-        }
-        catch (e) {
-            expect(e.message).toEqual(`Non-npm packages must not use names that conflict with existing npm packages.
+        })).toThrow(
+            `The non-npm package 'jquery' conflicts with the existing npm package 'jquery'.
 Try adding -browser to the end of the name to get
 
-    jquery-browser`)
-        }
+    jquery-browser
+`)
     }
 })
 suite("retrieveNpmHomepageOrFail", {
-    async retrieveFailure() {
-        expect.assertions(1)
-        try {
-            // surely parseltongue will never exist
-            await retrieveNpmHomepageOrFail("parseltongue")
-        }
-        catch (e) {
-            expect(e.message).toEqual(`404 - "{\\"error\\":\\"Not found\\"}"`)
-        }
+    retrieveFailure() {
+        // surely parseltongue will never exist
+        expect(() => retrieveNpmHomepageOrFail("parseltongue")).toThrow(`parseltongue Not found`);
     },
-    async retrieveShelljs() {
-        expect(await retrieveNpmHomepageOrFail("shelljs")).toBe("http://github.com/shelljs/shelljs")
+    retrieveShelljs() {
+        expect(retrieveNpmHomepageOrFail("shelljs")).toBe("http://github.com/shelljs/shelljs")
     }
 })
-suite("check", {
+suite("checkNames", {
     standaloneFail() {
-        expect(() => check({ dts: "a", src: "b" }, undefined)).toThrow("d.ts name 'a' must match source name 'b'.")
+        expect(() => checkNames({ dts: "a", src: "b" }, undefined)).toThrow("d.ts name 'a' must match source name 'b'.")
     },
     okWithJustHomepage() {
-        expect(check({ dts: "a", src: "a", homepage: "zombo.com" }, undefined)).toBeUndefined()
+        expect(checkNames({ dts: "a", src: "a", homepage: "zombo.com" }, undefined)).toBeUndefined()
     },
     okWithJustHeader() {
-        expect(check({ dts: "a", src: "a" }, {
+        expect(checkNames({ dts: "a", src: "a" }, {
             nonNpm: false,
@@ -176,3 +162,3 @@ libraryName: "a",
     homepageFail() {
-        expect(() => check({ dts: "a", src: "a", homepage: "zombo.com" }, {
+        expect(() => checkNames({ dts: "a", src: "a", homepage: "zombo.com" }, {
             nonNpm: false,
@@ -186,8 +172,18 @@ libraryName: "a",
         })).toThrow(`At least one of the project urls listed in the header, ["welcome-to-zombo.com","this-is-zombo.com"], must match the homepage listed by npm, 'zombo.com'.
-If your d.ts file is not for the npm package with zombo.com,
+If your d.ts file is not for the npm package with URL zombo.com,
 change the name by adding -browser to the end and change the first line
 of the Definitely Typed header to
 
-    // Type definitions for npn-npm package a-browser`)
+    // Type definitions for non-npm package a-browser`)
     }
 });
+
+suite("checkSource", {
+    badExportDefault() {
+        expect(() => checkSource(
+            "foo",
+            `function f() {}; export default f;`,
+            `module.exports = function () {};`)).toThrow(
+                "The types for foo specify 'export default' but the source does not mention 'default' anywhere.");
+    },
+});

package.json

 {
     "name": "dts-critic",
-    "version": "1.0.3",
+    "version": "1.0.4",
     "author": "Nathan Shively-Sanders",
     "description": "Checks a new .d.ts against the Javascript source and tells you what problems it has",
     "dependencies": {
-        "definitelytyped-header-parser": "^1.0.0",
-        "request-promise-native": "^1.0.5",
+        "definitelytyped-header-parser": "^1.0.1",
+        "download-file-sync": "^1.0.4",
         "yargs": "^12.0.5"
@@ -36,3 +36,2 @@ },
         "@types/node": "^10.12.21",
-        "@types/request-promise-native": "^1.0.15",
         "@types/strip-json-comments": "0.0.30",
@@ -39,0 +38,0 @@ "@types/yargs": "^12.0.8",

New alerts

Dynamic require

Supply chain risk

Dynamic require can indicate the package is performing dangerous or unsafe dynamic code execution.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

20008

increased by10.71%

Dev dependency count

6

decreased by-14.29%

Lines of code

449

increased by7.93%

Worsened metrics

Number of low supply chain risk alerts

3

increased by50%

Dependency changes

• + Addeddownload-file-sync@^1.0.4

• + Addeddownload-file-sync@1.0.4(transitive)

• - Removedrequest-promise-native@^1.0.5

• - Removedajv@6.12.6(transitive)

• - Removedasn1@0.2.6(transitive)

• - Removedassert-plus@1.0.0(transitive)

• - Removedasynckit@0.4.0(transitive)

• - Removedaws-sign2@0.7.0(transitive)

• - Removedaws4@1.13.2(transitive)

• - Removedbcrypt-pbkdf@1.0.2(transitive)

• - Removedcaseless@0.12.0(transitive)

• - Removedcombined-stream@1.0.8(transitive)

• - Removedcore-util-is@1.0.2(transitive)

• - Removeddashdash@1.14.1(transitive)

• - Removeddelayed-stream@1.0.0(transitive)

• - Removedecc-jsbn@0.1.2(transitive)

• - Removedextend@3.0.2(transitive)

• - Removedextsprintf@1.3.0(transitive)

• - Removedfast-deep-equal@3.1.3(transitive)

• - Removedfast-json-stable-stringify@2.1.0(transitive)

• - Removedforever-agent@0.6.1(transitive)

• - Removedform-data@2.3.3(transitive)

• - Removedgetpass@0.1.7(transitive)

• - Removedhar-schema@2.0.0(transitive)

• - Removedhar-validator@5.1.5(transitive)

• - Removedhttp-signature@1.2.0(transitive)

• - Removedis-typedarray@1.0.0(transitive)

• - Removedisstream@0.1.2(transitive)

• - Removedjsbn@0.1.1(transitive)

• - Removedjson-schema@0.4.0(transitive)

• - Removedjson-schema-traverse@0.4.1(transitive)

• - Removedjson-stringify-safe@5.0.1(transitive)

• - Removedjsprim@1.4.2(transitive)

• - Removedlodash@4.17.21(transitive)

• - Removedmime-db@1.52.0(transitive)

• - Removedmime-types@2.1.35(transitive)

• - Removedoauth-sign@0.9.0(transitive)

• - Removedperformance-now@2.1.0(transitive)

• - Removedpsl@1.15.0(transitive)

• - Removedpunycode@2.3.1(transitive)

• - Removedqs@6.5.3(transitive)

• - Removedrequest@2.88.2(transitive)

• - Removedrequest-promise-core@1.1.4(transitive)

• - Removedrequest-promise-native@1.0.9(transitive)

• - Removedsafe-buffer@5.2.1(transitive)

• - Removedsafer-buffer@2.1.2(transitive)

• - Removedsshpk@1.18.0(transitive)

• - Removedstealthy-require@1.1.1(transitive)

• - Removedtough-cookie@2.5.0(transitive)

• - Removedtunnel-agent@0.6.0(transitive)

• - Removedtweetnacl@0.14.5(transitive)

• - Removeduri-js@4.4.1(transitive)

• - Removeduuid@3.4.0(transitive)

• - Removedverror@1.10.0(transitive)

• Updateddefinitelytyped-header-parser@^1.0.1