Skip to content

Commit

Permalink
Remove react-scripts type reference on eject (facebook#5611)
Browse files Browse the repository at this point in the history
* Remove react-scripts type reference on eject

* Check for env file

* Check eject for typescript

* Shuffle appTypeDeclarations

* Append internal types on eject

* Ensure lib is published for types

* Adjust comment

* Don't add a bunch of new lines

* File should exist and not be deleted

* Add debug

* Set file explicitly

* Revert "Set file explicitly"

This reverts commit bcd58a3.

* Copy file before destroying ourselves

* Revert "Add debug"

This reverts commit 0068ba8.
  • Loading branch information
Timer authored Oct 30, 2018
1 parent 2a7fd5a commit 6364bbf
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 4 deletions.
7 changes: 4 additions & 3 deletions packages/react-scripts/config/paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand All @@ -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'),
Expand All @@ -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('lib/react-app.d.ts'),
};

const ownPackageJson = require('../package.json');
Expand All @@ -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'),
Expand All @@ -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('lib/react-app.d.ts'),
};
}
// @remove-on-eject-end
Expand Down
File renamed without changes.
3 changes: 2 additions & 1 deletion packages/react-scripts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"files": [
"bin",
"config",
"lib",
"scripts",
"template",
"template-typescript",
Expand All @@ -21,7 +22,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",
Expand Down
27 changes: 27 additions & 0 deletions packages/react-scripts/scripts/eject.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,33 @@ inquirer
);
console.log();

if (fs.existsSync(paths.appTypeDeclarations)) {
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 =
content
// Remove react-scripts types
.replace(
/^\s*\/\/\/\s*<reference\s+types.+?"react-scripts".*\/>.*(?:\n|$)/gm,
''
)
.trim() + os.EOL;

fs.writeFileSync(
paths.appTypeDeclarations,
(ownContent + os.EOL + content).trim() + os.EOL
);
} catch (e) {
// It's not essential that this succeeds, the TypeScript user should
// be able to re-create these types with ease.
}
}

// "Don't destroy what isn't ours"
if (ownPath.indexOf(appPath) === 0) {
try {
Expand Down
14 changes: 14 additions & 0 deletions tasks/e2e-installs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -170,13 +170,27 @@ 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
yarn start --smoke-test
yarn build
CI=true yarn test

# Check eject behaves and works

# Eject...
echo yes | npm run eject

# 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
yarn build
CI=true yarn test

# ******************************************************************************
# Test --scripts-version with a tarball url
# ******************************************************************************
Expand Down

0 comments on commit 6364bbf

Please sign in to comment.