From bf782b906f3bcafe82f736a0d73bc97576209d16 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Sun, 28 Oct 2018 20:11:48 -0400 Subject: [PATCH 01/14] Remove react-scripts type reference on eject --- packages/react-scripts/scripts/eject.js | 29 +++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/react-scripts/scripts/eject.js b/packages/react-scripts/scripts/eject.js index 84353fc3c81..a39342e8f6e 100644 --- a/packages/react-scripts/scripts/eject.js +++ b/packages/react-scripts/scripts/eject.js @@ -236,6 +236,35 @@ inquirer } } + if (fs.existsSync(paths.appTypeDeclarations)) { + try { + // Read app declarations file + let content = fs.readFileSync(paths.appTypeDeclarations, 'utf8'); + + // Remove react-scripts reference since they're getting a copy of the types in their project + content = + content + // Remove react-scripts types + .replace( + /^\s*\/\/\/\s*.*(?:\n|$)/gm, + '' + ) + .trim() + os.EOL; + + if (content === os.EOL) { + // Remove the file if it is empty + fs.removeSync(paths.appTypeDeclarations); + } else { + // The user probably added their own additional types, so let's just + // write it without ours + fs.writeFileSync(paths.appTypeDeclarations, content); + } + } catch (e) { + // It's not essential that this succeeds, + // The user should know to clean this up + } + } + if (fs.existsSync(paths.yarnLockFile)) { const windowsCmdFilePath = path.join( appPath, From db33ddd06f5739bb7a1540b0499588af68e5c941 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Sun, 28 Oct 2018 21:56:50 -0400 Subject: [PATCH 02/14] Check for env file --- tasks/e2e-installs.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tasks/e2e-installs.sh b/tasks/e2e-installs.sh index 690a9cd04fd..0eaa7ec6ab8 100755 --- a/tasks/e2e-installs.sh +++ b/tasks/e2e-installs.sh @@ -170,6 +170,7 @@ exists node_modules/react-scripts exists node_modules/typescript exists src/index.tsx exists tsconfig.json +exists src/react-app-env.d.ts checkTypeScriptDependencies # Check that the TypeScript template passes smoke tests, build, and normal tests From c02aa0895619c89106596f4decaf7b0ec520bdf0 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Sun, 28 Oct 2018 21:58:02 -0400 Subject: [PATCH 03/14] Check eject for typescript --- tasks/e2e-installs.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tasks/e2e-installs.sh b/tasks/e2e-installs.sh index 0eaa7ec6ab8..a2438bf8547 100755 --- a/tasks/e2e-installs.sh +++ b/tasks/e2e-installs.sh @@ -178,6 +178,19 @@ yarn start --smoke-test yarn build CI=true yarn test +# Check eject behaves and works + +# Eject... +echo yes | npm run eject + +# Ensure env file was removed +! exists src/react-app-env.d.ts + +# Check that the TypeScript template passes ejected smoke tests, build, and normal tests +yarn start --smoke-test +yarn build +CI=true yarn test + # ****************************************************************************** # Test --scripts-version with a tarball url # ****************************************************************************** From 3d3925927fdd06e6e0ab061249cd76c092633b4a Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Sun, 28 Oct 2018 23:27:20 -0400 Subject: [PATCH 04/14] Shuffle appTypeDeclarations --- packages/react-scripts/config/paths.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/react-scripts/config/paths.js b/packages/react-scripts/config/paths.js index 70fc76a0140..61ffe2c4f4e 100644 --- a/packages/react-scripts/config/paths.js +++ b/packages/react-scripts/config/paths.js @@ -84,7 +84,6 @@ module.exports = { appPackageJson: resolveApp('package.json'), appSrc: resolveApp('src'), appTsConfig: resolveApp('tsconfig.json'), - appTypeDeclarations: resolveApp('src/react-app-env.d.ts'), yarnLockFile: resolveApp('yarn.lock'), testsSetup: resolveModule(resolveApp, 'src/setupTests'), proxySetup: resolveApp('src/setupProxy.js'), @@ -107,7 +106,6 @@ module.exports = { appPackageJson: resolveApp('package.json'), appSrc: resolveApp('src'), appTsConfig: resolveApp('tsconfig.json'), - appTypeDeclarations: resolveApp('src/react-app-env.d.ts'), yarnLockFile: resolveApp('yarn.lock'), testsSetup: resolveModule(resolveApp, 'src/setupTests'), proxySetup: resolveApp('src/setupProxy.js'), @@ -117,6 +115,8 @@ module.exports = { // These properties only exist before ejecting: ownPath: resolveOwn('.'), ownNodeModules: resolveOwn('node_modules'), // This is empty on npm 3 + appTypeDeclarations: resolveApp('src/react-app-env.d.ts'), + ownTypeDeclarations: resolveOwn('config/react-app.d.ts'), }; const ownPackageJson = require('../package.json'); @@ -140,7 +140,6 @@ if ( appPackageJson: resolveOwn('package.json'), appSrc: resolveOwn('template/src'), appTsConfig: resolveOwn('template/tsconfig.json'), - appTypeDeclarations: resolveOwn('template/src/react-app-env.d.ts'), yarnLockFile: resolveOwn('template/yarn.lock'), testsSetup: resolveModule(resolveOwn, 'template/src/setupTests'), proxySetup: resolveOwn('template/src/setupProxy.js'), @@ -150,6 +149,8 @@ if ( // These properties only exist before ejecting: ownPath: resolveOwn('.'), ownNodeModules: resolveOwn('node_modules'), + appTypeDeclarations: resolveOwn('template/src/react-app-env.d.ts'), + ownTypeDeclarations: resolveOwn('config/react-app.d.ts'), }; } // @remove-on-eject-end From a8dc1a555753ff4fa49107ec0620eefbb6166580 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Sun, 28 Oct 2018 23:31:58 -0400 Subject: [PATCH 05/14] Append internal types on eject --- packages/react-scripts/config/paths.js | 4 ++-- .../react-scripts/{config => lib}/react-app.d.ts | 0 packages/react-scripts/package.json | 2 +- packages/react-scripts/scripts/eject.js | 14 ++++++-------- 4 files changed, 9 insertions(+), 11 deletions(-) rename packages/react-scripts/{config => lib}/react-app.d.ts (100%) diff --git a/packages/react-scripts/config/paths.js b/packages/react-scripts/config/paths.js index 61ffe2c4f4e..b719054583b 100644 --- a/packages/react-scripts/config/paths.js +++ b/packages/react-scripts/config/paths.js @@ -116,7 +116,7 @@ module.exports = { ownPath: resolveOwn('.'), ownNodeModules: resolveOwn('node_modules'), // This is empty on npm 3 appTypeDeclarations: resolveApp('src/react-app-env.d.ts'), - ownTypeDeclarations: resolveOwn('config/react-app.d.ts'), + ownTypeDeclarations: resolveOwn('lib/react-app.d.ts'), }; const ownPackageJson = require('../package.json'); @@ -150,7 +150,7 @@ if ( ownPath: resolveOwn('.'), ownNodeModules: resolveOwn('node_modules'), appTypeDeclarations: resolveOwn('template/src/react-app-env.d.ts'), - ownTypeDeclarations: resolveOwn('config/react-app.d.ts'), + ownTypeDeclarations: resolveOwn('lib/react-app.d.ts'), }; } // @remove-on-eject-end diff --git a/packages/react-scripts/config/react-app.d.ts b/packages/react-scripts/lib/react-app.d.ts similarity index 100% rename from packages/react-scripts/config/react-app.d.ts rename to packages/react-scripts/lib/react-app.d.ts diff --git a/packages/react-scripts/package.json b/packages/react-scripts/package.json index d6cea812012..91b684528fb 100644 --- a/packages/react-scripts/package.json +++ b/packages/react-scripts/package.json @@ -21,7 +21,7 @@ "bin": { "react-scripts": "./bin/react-scripts.js" }, - "types": "./config/react-app.d.ts", + "types": "./lib/react-app.d.ts", "dependencies": { "@babel/core": "7.1.0", "@svgr/webpack": "2.4.1", diff --git a/packages/react-scripts/scripts/eject.js b/packages/react-scripts/scripts/eject.js index a39342e8f6e..25671fe0142 100644 --- a/packages/react-scripts/scripts/eject.js +++ b/packages/react-scripts/scripts/eject.js @@ -240,6 +240,8 @@ inquirer try { // Read app declarations file let content = fs.readFileSync(paths.appTypeDeclarations, 'utf8'); + const ownContent = + fs.readFileSync(paths.ownTypeDeclarations, 'utf8').trim() + os.EOL; // Remove react-scripts reference since they're getting a copy of the types in their project content = @@ -251,14 +253,10 @@ inquirer ) .trim() + os.EOL; - if (content === os.EOL) { - // Remove the file if it is empty - fs.removeSync(paths.appTypeDeclarations); - } else { - // The user probably added their own additional types, so let's just - // write it without ours - fs.writeFileSync(paths.appTypeDeclarations, content); - } + fs.writeFileSync( + paths.appTypeDeclarations, + ownContent + os.EOL + content + ); } catch (e) { // It's not essential that this succeeds, // The user should know to clean this up From 10fd26132753b80f1c927d0e2dd39274fe5bbdb0 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Sun, 28 Oct 2018 23:35:46 -0400 Subject: [PATCH 06/14] Ensure lib is published for types --- packages/react-scripts/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-scripts/package.json b/packages/react-scripts/package.json index 91b684528fb..15b8801b121 100644 --- a/packages/react-scripts/package.json +++ b/packages/react-scripts/package.json @@ -13,6 +13,7 @@ "files": [ "bin", "config", + "lib", "scripts", "template", "template-typescript", From 37a696d63b815b15fe6d0c80466d72754f9677e7 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Sun, 28 Oct 2018 23:39:39 -0400 Subject: [PATCH 07/14] Adjust comment --- packages/react-scripts/scripts/eject.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-scripts/scripts/eject.js b/packages/react-scripts/scripts/eject.js index 25671fe0142..c35afdc6282 100644 --- a/packages/react-scripts/scripts/eject.js +++ b/packages/react-scripts/scripts/eject.js @@ -258,8 +258,8 @@ inquirer ownContent + os.EOL + content ); } catch (e) { - // It's not essential that this succeeds, - // The user should know to clean this up + // It's not essential that this succeeds, the TypeScript user should + // be able to re-create these types with ease. } } From 272da12caabdd9a3c4ada987ff432545c6b948c9 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Mon, 29 Oct 2018 00:04:39 -0400 Subject: [PATCH 08/14] Don't add a bunch of new lines --- packages/react-scripts/scripts/eject.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-scripts/scripts/eject.js b/packages/react-scripts/scripts/eject.js index c35afdc6282..8f8cfcb1a14 100644 --- a/packages/react-scripts/scripts/eject.js +++ b/packages/react-scripts/scripts/eject.js @@ -255,7 +255,7 @@ inquirer fs.writeFileSync( paths.appTypeDeclarations, - ownContent + os.EOL + content + (ownContent + os.EOL + content).trim() + os.EOL ); } catch (e) { // It's not essential that this succeeds, the TypeScript user should From 6ee2925c129c757ed50ee38d4dff6d0032b1bcf0 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Mon, 29 Oct 2018 00:08:40 -0400 Subject: [PATCH 09/14] File should exist and not be deleted --- tasks/e2e-installs.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tasks/e2e-installs.sh b/tasks/e2e-installs.sh index a2438bf8547..1be1c18468b 100755 --- a/tasks/e2e-installs.sh +++ b/tasks/e2e-installs.sh @@ -183,8 +183,8 @@ CI=true yarn test # Eject... echo yes | npm run eject -# Ensure env file was removed -! exists src/react-app-env.d.ts +# Ensure env file still exists +exists src/react-app-env.d.ts # Check that the TypeScript template passes ejected smoke tests, build, and normal tests yarn start --smoke-test From 0068ba81c6d79d99788877c9e1b618acd7412dce Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Mon, 29 Oct 2018 17:44:59 -0400 Subject: [PATCH 10/14] Add debug --- packages/react-scripts/scripts/eject.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-scripts/scripts/eject.js b/packages/react-scripts/scripts/eject.js index 8f8cfcb1a14..08d79e779c8 100644 --- a/packages/react-scripts/scripts/eject.js +++ b/packages/react-scripts/scripts/eject.js @@ -258,6 +258,7 @@ inquirer (ownContent + os.EOL + content).trim() + os.EOL ); } catch (e) { + console.error('error setting types: ' + e); // It's not essential that this succeeds, the TypeScript user should // be able to re-create these types with ease. } From bcd58a36cbd08a71af50b037d8f1fae6c595fb4e Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Mon, 29 Oct 2018 19:53:06 -0400 Subject: [PATCH 11/14] Set file explicitly --- packages/react-scripts/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-scripts/package.json b/packages/react-scripts/package.json index 15b8801b121..92aefa64fe3 100644 --- a/packages/react-scripts/package.json +++ b/packages/react-scripts/package.json @@ -13,7 +13,7 @@ "files": [ "bin", "config", - "lib", + "lib/react-app.d.ts", "scripts", "template", "template-typescript", From 061062c9ac769449798ff63f72f3589eb298c6a8 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Mon, 29 Oct 2018 21:39:50 -0400 Subject: [PATCH 12/14] Revert "Set file explicitly" This reverts commit bcd58a36cbd08a71af50b037d8f1fae6c595fb4e. --- packages/react-scripts/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-scripts/package.json b/packages/react-scripts/package.json index 92aefa64fe3..15b8801b121 100644 --- a/packages/react-scripts/package.json +++ b/packages/react-scripts/package.json @@ -13,7 +13,7 @@ "files": [ "bin", "config", - "lib/react-app.d.ts", + "lib", "scripts", "template", "template-typescript", From 544b52782d299c61b6379da07987f7e4c2d78059 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Mon, 29 Oct 2018 22:24:05 -0400 Subject: [PATCH 13/14] Copy file before destroying ourselves --- packages/react-scripts/scripts/eject.js | 26 ++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/react-scripts/scripts/eject.js b/packages/react-scripts/scripts/eject.js index 08d79e779c8..dbcc4ca630f 100644 --- a/packages/react-scripts/scripts/eject.js +++ b/packages/react-scripts/scripts/eject.js @@ -223,19 +223,6 @@ inquirer ); console.log(); - // "Don't destroy what isn't ours" - if (ownPath.indexOf(appPath) === 0) { - try { - // remove react-scripts and react-scripts binaries from app node_modules - Object.keys(ownPackage.bin).forEach(binKey => { - fs.removeSync(path.join(appPath, 'node_modules', '.bin', binKey)); - }); - fs.removeSync(ownPath); - } catch (e) { - // It's not essential that this succeeds - } - } - if (fs.existsSync(paths.appTypeDeclarations)) { try { // Read app declarations file @@ -264,6 +251,19 @@ inquirer } } + // "Don't destroy what isn't ours" + if (ownPath.indexOf(appPath) === 0) { + try { + // remove react-scripts and react-scripts binaries from app node_modules + Object.keys(ownPackage.bin).forEach(binKey => { + fs.removeSync(path.join(appPath, 'node_modules', '.bin', binKey)); + }); + fs.removeSync(ownPath); + } catch (e) { + // It's not essential that this succeeds + } + } + if (fs.existsSync(paths.yarnLockFile)) { const windowsCmdFilePath = path.join( appPath, From f32b217a9de4e93d78a83efd4be9fc92d07f359c Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Mon, 29 Oct 2018 22:24:17 -0400 Subject: [PATCH 14/14] Revert "Add debug" This reverts commit 0068ba81c6d79d99788877c9e1b618acd7412dce. --- packages/react-scripts/scripts/eject.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react-scripts/scripts/eject.js b/packages/react-scripts/scripts/eject.js index dbcc4ca630f..f4dba4d0b0f 100644 --- a/packages/react-scripts/scripts/eject.js +++ b/packages/react-scripts/scripts/eject.js @@ -245,7 +245,6 @@ inquirer (ownContent + os.EOL + content).trim() + os.EOL ); } catch (e) { - console.error('error setting types: ' + e); // It's not essential that this succeeds, the TypeScript user should // be able to re-create these types with ease. }