Skip to content

add support for installing react-scripts using npm link#1122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tabrezm wants to merge 3 commits into from
+29 −17
Closed

add support for installing react-scripts using npm link #1122

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ee55f5
fix module.exports for react-scripts
Dec 1, 2016
fd4f81b
don't hardcode react-scripts module name
Dec 1, 2016
737c20b
Update paths.js
Dec 1, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 29 additions & 17 deletions packages/react-scripts/config/paths.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -55,24 +55,36 @@ function resolveOwn(relativePath) {
return path.resolve(__dirname, relativePath);
}

// config before eject: we're in ./node_modules/react-scripts/config/
module.exports = {
appBuild: resolveApp('build'),
appPublic: resolveApp('public'),
appHtml: resolveApp('public/index.html'),
appIndexJs: resolveApp('src/index.js'),
appPackageJson: resolveApp('package.json'),
appSrc: resolveApp('src'),
yarnLockFile: resolveApp('yarn.lock'),
testsSetup: resolveApp('src/setupTests.js'),
appNodeModules: resolveApp('node_modules'),
// this is empty with npm3 but node resolution searches higher anyway:
ownNodeModules: resolveOwn('../node_modules'),
nodePaths: nodePaths
};
// Set up module.exports depending on how react-scripts is run. The current
// path will contain `packages/react-scripts` when running from create-react-app.
// We don't make assumptions about the path when this package is installed from
// another app since it could've been renamed.

// Note that when installing this package using `npm link`, the current
// directory path contains `packages/react-scripts` (instead of, for example,
// `node_modules/react-scripts`). There is a specific condition to handle this
// case so that it doesn't conflict with other scripts (like `build` or
// `publish`) or running the smoke test.
var isRunningFromOwn = __dirname.indexOf(path.join('packages', 'react-scripts', 'config')) !== -1;
var isSmokeTest = process.argv.some(arg => arg.indexOf('--smoke-test') > -1);
var isRunningFromAppUsingLink = (process.env.npm_lifecycle_event === 'start') && isRunningFromOwn && !isSmokeTest

@gaearongaearonDec 12, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is special about start? I don't understand this.

@tabrezmtabrezmDec 12, 2016

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without including the check for start, (i.e. only checking isRunningFromOwn && !isSmokeTest), the tests fail because those conditions are both true for other events like build. At first glance, it may be assuming too much, as it's entirely possible that someone will write an app that has other start-like events, but are named something else. But they should be able to handle their own custom cases since they'll be forking react-scripts in the first place.


// config before publish: we're in ./packages/react-scripts/config/
if (__dirname.indexOf(path.join('packages', 'react-scripts', 'config')) !== -1) {
if (!isRunningFromOwn || isRunningFromAppUsingLink) {
module.exports = {
appBuild: resolveApp('build'),
appPublic: resolveApp('public'),
appHtml: resolveApp('public/index.html'),
appIndexJs: resolveApp('src/index.js'),
appPackageJson: resolveApp('package.json'),
appSrc: resolveApp('src'),
yarnLockFile: resolveApp('yarn.lock'),
testsSetup: resolveApp('src/setupTests.js'),
appNodeModules: resolveApp('node_modules'),
// this is empty with npm3 but node resolution searches higher anyway:
ownNodeModules: resolveOwn('../node_modules'),
nodePaths: nodePaths
};
} else {
module.exports = {
appBuild: resolveOwn('../../../build'),
appPublic: resolveOwn('../template/public'),
Expand Down