(fix): set rootDir to './src', not './'. deprecate moveTypes (#504)

* (fix/refactor): rewrite some overbroad try/catches - so there's less silent failures that occur but don't error - replace overbroad try/catch in getProjectPath with just an fs.pathExists - replace overbroad try/catch in cleanDistFolder with just an fs.remove - fs.remove is like rimraf and `rm -rf` in that it won't error if the file/dir doesn't exist - if it does error, it's probably because it *failed* to remove the dir, and that should error, because it's potentially a problem, especially if you're publishing right after - rewrite moveTypes() so it doesn't have an overbroad try/catch - use fs.pathExists first and early return if it doesn't exist - only check for known errors with fs.copy, and rethrow others - this way if copy or remove actually fail, they will actually error - before they would silently fail, which could similarly be pretty bad if one were to publish right after a silent failure * (fix): set rootDir to './src', not './'. deprecate moveTypes - rootDir needed to be changed to ./src because the previous ./ caused type declarations to be generated in dist/src/ instead of just dist/ - the moveTypes function handled moving the declarations back into dist/, but occassionally had errors moving .d.ts files - particularly in CI and for projects with many of them - declarationMap (*.d.ts.map) files would also have issues due to the hackiness of moveTypes, setting to rootDir to './src' is one of the necessary steps in fixing them - deprecate moveTypes and add a warning with instructions if it is used when a rootDir of './' is detected - add notes about a deprecation window in the comments * (empty/removeme): test CI again 1 * (empty/removeme): test CI again 2 * (empty/removeme): test CI again 3 * (empty/removeme): test CI again 4 * (empty/removeme): test CI again 5 * (empty/removeme): test CI again 6 * (empty/removeme): test CI again 7 * (empty/removeme): test CI again 8 * (empty/removeme): test CI again 9 * (empty/removeme): test CI again 10 * (empty/removeme): test CI again 11 * (empty/removeme): test CI again 12 * (empty/removeme): test CI again 13 * (empty/removeme): test CI again 14 * (empty/removeme): test CI again 15 * (empty/removeme): test CI again 16 * (empty/removeme): test CI again 17 * (empty/removeme): test CI again 18 * (empty/removeme): test CI again 19 * (empty/removeme): test CI again 20 * (empty/removeme): test CI again 21 * (empty/removeme): test CI again 22 * (empty/removeme): test CI again 23 * (empty/removeme): test CI again 24 * (empty/removeme): test CI again 25 * (empty/removeme): test CI again 26 * (empty/removeme): test CI again 27 * (empty/removeme): test CI again 28 * (empty/removeme): test CI again 29 * (empty/removeme): test CI again 30 * (empty/removeme): test CI again 31 * (empty/removeme): test CI again 32 * (empty/removeme): test CI again 33 * more descriptive warning about bugs, fixup with (fix): set rootDir to './src', not './'. deprecate moveTypes * (empty/removeme): test CI again 34 * (empty/removeme): test CI again 35 * (empty/removeme): test CI again 36 * (empty/removeme): test CI again 37 * (empty/removeme): test CI again 38 * add a comment that the catch is the problem, fixup with (fix): set rootDir to './src', not './'. deprecate moveTypes
jaredpalmer · Feb 12, 2020 · 68f26c9 · 68f26c9
1 parent a97a5af
commit 68f26c9
Show file tree

Hide file tree

Showing 8 changed files with 58 additions and 35 deletions.
diff --git a/src/deprecated.ts b/src/deprecated.ts
@@ -0,0 +1,45 @@
+import * as fs from 'fs-extra';
+
+import { paths } from './constants';
+
+/*
+  This was originally needed because the default
+  tsconfig.compilerOptions.rootDir was set to './' instead of './src'.
+  Now that it's set to './src', this is now deprecated.
+  To ensure a stable upgrade path for users, leave the warning in for
+  6 months - 1 year, then change it to an error in a breaking bump and leave
+  that in for some time too.
+*/
+export async function moveTypes() {
+  const appDistSrc = paths.appDist + '/src';
+
+  const pathExists = await fs.pathExists(appDistSrc);
+  if (!pathExists) return;
+
+  // see note above about deprecation window
+  console.warn(
+    '[tsdx]: Your rootDir is currently set to "./". Please change your ' +
+      'rootDir to "./src".\n' +
+      'TSDX has deprecated setting tsconfig.compilerOptions.rootDir to ' +
+      '"./" as it caused buggy output for declarationMaps and occassionally ' +
+      'for type declarations themselves.'
+  );
+
+  try {
+    // Move the typescript types to the base of the ./dist folder
+    await fs.copy(appDistSrc, paths.appDist, {
+      overwrite: true,
+    });
+  } catch (err) {
+    // ignore errors about the destination dir already existing or files not
+    // existing as those always occur for some reason, re-throw any other
+    // unexpected failures
+    // NOTE: these errors mean that sometimes files don't get moved properly,
+    // meaning that it's buggy / unreliable (see console.warn above)
+    if (err.code !== 'EEXIST' && err.code !== 'ENOENT') {
+      throw err;
+    }
+  }
+
+  await fs.remove(appDistSrc);
+}
diff --git a/src/index.ts b/src/index.ts
@@ -12,13 +12,11 @@ import {
 } from 'rollup';
 import asyncro from 'asyncro';
 import chalk from 'chalk';
-import util from 'util';
 import * as fs from 'fs-extra';
 import jest from 'jest';
 import { CLIEngine } from 'eslint';
 import logError from './logError';
 import path from 'path';
-import rimraf from 'rimraf';
 import execa from 'execa';
 import shell from 'shelljs';
 import ora from 'ora';
@@ -48,6 +46,7 @@ import {
 import { createProgressEstimator } from './createProgressEstimator';
 import { templates } from './templates';
 import { composePackageJson } from './templates/utils';
+import * as deprecated from './deprecated';
 const pkg = require('../package.json');
 
 const prog = sade('tsdx');
@@ -101,16 +100,6 @@ async function getInputs(
   return concatAllArray(inputs);
 }
 
-async function moveTypes() {
-  try {
-    // Move the typescript types to the base of the ./dist folder
-    await fs.copy(paths.appDist + '/src', paths.appDist, {
-      overwrite: true,
-    });
-    await fs.remove(paths.appDist + '/src');
-  } catch (e) {}
-}
-
 prog
   .version(pkg.version)
   .command('create <pkg>')
@@ -140,16 +129,11 @@ prog
     // Helper fn to prompt the user for a different
     // folder name if one already exists
     async function getProjectPath(projectPath: string): Promise<string> {
-      let exists = true;
-      try {
-        // will throw an exception if it does not exists
-        await util.promisify(fs.access)(projectPath);
-      } catch {
-        exists = false;
-      }
+      const exists = await fs.pathExists(projectPath);
       if (!exists) {
         return projectPath;
       }
+
       bootSpinner.fail(`Failed to create ${chalk.bold.red(pkg)}`);
       const prompt = new Input({
         message: `A folder named ${chalk.bold.red(
@@ -158,6 +142,7 @@ prog
         initial: pkg + '-1',
         result: (v: string) => v.trim(),
       });
+
       pkg = await prompt.run();
       projectPath = (await fs.realpath(process.cwd())) + '/' + pkg;
       bootSpinner.start(`Creating ${chalk.bold.green(pkg)}...`);
@@ -373,7 +358,7 @@ prog
 `);
 
         try {
-          await moveTypes();
+          await deprecated.moveTypes();
 
           if (firstTime && opts.onFirstSuccess) {
             firstTime = false;
@@ -424,7 +409,7 @@ prog
           async (inputOptions: RollupOptions & { output: OutputOptions }) => {
             let bundle = await rollup(inputOptions);
             await bundle.write(inputOptions.output);
-            await moveTypes();
+            await deprecated.moveTypes();
           }
         )
         .catch((e: any) => {
@@ -453,14 +438,7 @@ async function normalizeOpts(opts: WatchOpts): Promise<NormalizedOpts> {
 }
 
 async function cleanDistFolder() {
-  try {
-    await util.promisify(fs.access)(paths.appDist);
-    return util.promisify(rimraf)(paths.appDist);
-  } catch {
-    // if an exception is throw, the files does not exists or it is not visible
-    // either way, we just return
-    return;
-  }
+  await fs.remove(paths.appDist);
 }
 
 function writeCjsEntryFile(name: string) {

diff --git a/templates/basic/tsconfig.json b/templates/basic/tsconfig.json
@@ -6,7 +6,7 @@
     "importHelpers": true,
     "declaration": true,
     "sourceMap": true,
-    "rootDir": "./",
+    "rootDir": "./src",
     "strict": true,
     "noImplicitAny": true,
     "strictNullChecks": true,

diff --git a/templates/react-with-storybook/tsconfig.json b/templates/react-with-storybook/tsconfig.json
@@ -6,7 +6,7 @@
     "importHelpers": true,
     "declaration": true,
     "sourceMap": true,
-    "rootDir": "./",
+    "rootDir": "./src",
     "strict": true,
     "noImplicitAny": true,
     "strictNullChecks": true,

diff --git a/templates/react/tsconfig.json b/templates/react/tsconfig.json
@@ -6,7 +6,7 @@
     "importHelpers": true,
     "declaration": true,
     "sourceMap": true,
-    "rootDir": "./",
+    "rootDir": "./src",
     "strict": true,
     "noImplicitAny": true,
     "strictNullChecks": true,

diff --git a/test/fixtures/build-default/tsconfig.json b/test/fixtures/build-default/tsconfig.json
@@ -4,7 +4,7 @@
     "lib": ["dom", "esnext"],
     "declaration": true,
     "sourceMap": true,
-    "rootDir": "./",
+    "rootDir": "./src",
     "strict": true,
     "noImplicitAny": true,
     "strictNullChecks": true,

diff --git a/test/fixtures/build-invalid/tsconfig.json b/test/fixtures/build-invalid/tsconfig.json
@@ -4,7 +4,7 @@
     "lib": ["dom", "esnext"],
     "declaration": true,
     "sourceMap": true,
-    "rootDir": "./",
+    "rootDir": "./src",
     "strict": true,
     "noImplicitAny": true,
     "strictNullChecks": true,

diff --git a/test/fixtures/build-withConfig/tsconfig.json b/test/fixtures/build-withConfig/tsconfig.json
@@ -4,7 +4,7 @@
     "lib": ["dom", "esnext"],
     "declaration": true,
     "sourceMap": true,
-    "rootDir": "./",
+    "rootDir": "./src",
     "strict": true,
     "noImplicitAny": true,
     "strictNullChecks": true,