Skip to content
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.

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

Set baseUrl from jsconfig.json/tsconfig.json #6656

Merged
merged 22 commits into from
Apr 16, 2019
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
13f65e8
Set baseUrl from jsconfig.json/tsconfig.json
rovansteen Mar 15, 2019
3237f4c
Resolve the path for loading modules
rovansteen Mar 15, 2019
8d35263
Add tests for jsconfig.json
rovansteen Mar 15, 2019
7ab30a4
Add jsconfig.json
rovansteen Mar 15, 2019
503bd63
Update packages/react-scripts/scripts/start.js
iansu Mar 15, 2019
4dd6005
Move baseUrl test to config folder
rovansteen Mar 15, 2019
d7f0816
Remove alias test
rovansteen Mar 15, 2019
b16bad2
Use chalk from react-dev-utils
rovansteen Mar 17, 2019
9eca658
Add lost absolute file for typescript baseUrl test
rovansteen Mar 17, 2019
a7b084f
Update packages/react-scripts/config/modules.js
mrmckeb Apr 1, 2019
8193cef
Update other references of useTypeScript to hasTsConfig
rovansteen Apr 1, 2019
92af347
Merge branch 'master' into typescript-base-url
Apr 4, 2019
4f627e9
Fix casing of TypeScript
iansu Apr 5, 2019
c629a9e
Keep respecting NODE_PATH for now to support multiple module paths.
rovansteen Apr 14, 2019
359a0b2
Merge remote-tracking branch 'upstream/master' into typescript-base-url
rovansteen Apr 14, 2019
e72d38c
Add test for NODE_PATH
rovansteen Apr 14, 2019
84a4792
Add fallback if NODE_PATH is not set.
rovansteen Apr 14, 2019
0690231
Fix node path behavior tests
iansu Apr 16, 2019
1d8f29c
Merge branch 'master' into typescript-base-url
iansu Apr 16, 2019
255a6bd
Remove debugging code from behavior test suite
iansu Apr 16, 2019
0e0b19e
Remove more debugging code
iansu Apr 16, 2019
f039bf5
Show NODE_PATH deprecation warning during build
iansu Apr 16, 2019
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
17 changes: 0 additions & 17 deletions packages/react-scripts/config/env.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
'use strict';

const fs = require('fs');
const path = require('path');
const paths = require('./paths');

// Make sure that including paths.js after env.js will read .env variables.
Expand Down Expand Up @@ -48,22 +47,6 @@ dotenvFiles.forEach(dotenvFile => {
}
});

// We support resolving modules according to `NODE_PATH`.
// This lets you use absolute paths in imports inside large monorepos:
// https://github.com/facebook/create-react-app/issues/253.
// It works similar to `NODE_PATH` in Node itself:
// https://nodejs.org/api/modules.html#modules_loading_from_the_global_folders
// Note that unlike in Node, only *relative* paths from `NODE_PATH` are honored.
// Otherwise, we risk importing Node.js core modules into an app instead of Webpack shims.
// https://github.com/facebook/create-react-app/issues/1023#issuecomment-265344421
// We also resolve them to make sure all tools using them work consistently.
const appDirectory = fs.realpathSync(process.cwd());
process.env.NODE_PATH = (process.env.NODE_PATH || '')
.split(path.delimiter)
.filter(folder => folder && !path.isAbsolute(folder))
.map(folder => path.resolve(appDirectory, folder))
.join(path.delimiter);

// Grab NODE_ENV and REACT_APP_* environment variables and prepare them to be
// injected into the application via DefinePlugin in Webpack configuration.
const REACT_APP = /^REACT_APP_/i;
Expand Down
87 changes: 87 additions & 0 deletions packages/react-scripts/config/modules.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// @remove-on-eject-begin
/**
* Copyright (c) 2015-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
// @remove-on-eject-end
'use strict';

const fs = require('fs');
const path = require('path');
const paths = require('./paths');
const chalk = require('react-dev-utils/chalk');

/**
* Get the baseUrl of a compilerOptions object.
*
* @param {Object} options
*/
function getAdditionalModulePath(options = {}) {
const baseUrl = options.baseUrl;

// We need to explicitly check for null and undefined (and not a falsy value) because
// TypeScript treats an empty string as `.`.
if (baseUrl == null) {
return null;
}

const baseUrlResolved = path.resolve(paths.appPath, baseUrl);

// We don't need to do anything if `baseUrl` is set to `node_modules`. This is
// the default behavior.
if (path.relative(paths.appNodeModules, baseUrlResolved) === '') {
return null;
}

// Allow the user set the `baseUrl` to `appSrc`.
if (path.relative(paths.appSrc, baseUrlResolved) === '') {
return paths.appSrc;
}

// Otherwise, throw an error.
throw new Error(
chalk.red.bold(
"Your project's `baseUrl` can only be set to `src` or `node_modules`." +
' Create React App does not support other values at this time.'
)
);
}

function getModules() {
// Check if TypeScript is setup
const useTypeScript = fs.existsSync(paths.appTsConfig);
rovansteen marked this conversation as resolved.
Show resolved Hide resolved
const hasJsConfig = fs.existsSync(paths.appJsConfig);

if (useTypeScript && hasJsConfig) {
throw new Error(
'You have both a tsconfig.json and a jsconfig.json. If you are using Typescript please remove your jsconfig.json file.'
);
}

let config;

// If there's a tsconfig.json we assume it's a
// Typescript project and set up the config
// based on tsconfig.json
if (useTypeScript) {
config = require(paths.appTsConfig);
// Otherwise we'll check if there is jsconfig.json
// for non TS projects.
} else if (hasJsConfig) {
config = require(paths.appJsConfig);
}

config = config || {};
const options = config.compilerOptions || {};

const additionalModulePath = getAdditionalModulePath(options);

return {
additionalModulePath: additionalModulePath,
useTypeScript,
};
}

module.exports = getModules();
3 changes: 3 additions & 0 deletions packages/react-scripts/config/paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ module.exports = {
appPackageJson: resolveApp('package.json'),
appSrc: resolveApp('src'),
appTsConfig: resolveApp('tsconfig.json'),
appJsConfig: resolveApp('jsconfig.json'),
yarnLockFile: resolveApp('yarn.lock'),
testsSetup: resolveModule(resolveApp, 'src/setupTests'),
proxySetup: resolveApp('src/setupProxy.js'),
Expand All @@ -106,6 +107,7 @@ module.exports = {
appPackageJson: resolveApp('package.json'),
appSrc: resolveApp('src'),
appTsConfig: resolveApp('tsconfig.json'),
appJsConfig: resolveApp('jsconfig.json'),
yarnLockFile: resolveApp('yarn.lock'),
testsSetup: resolveModule(resolveApp, 'src/setupTests'),
proxySetup: resolveApp('src/setupProxy.js'),
Expand Down Expand Up @@ -140,6 +142,7 @@ if (
appPackageJson: resolveOwn('package.json'),
appSrc: resolveOwn('template/src'),
appTsConfig: resolveOwn('template/tsconfig.json'),
appJsConfig: resolveOwn('template/jsconfig.json'),
yarnLockFile: resolveOwn('template/yarn.lock'),
testsSetup: resolveModule(resolveOwn, 'template/src/setupTests'),
proxySetup: resolveOwn('template/src/setupProxy.js'),
Expand Down
4 changes: 2 additions & 2 deletions packages/react-scripts/config/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const WatchMissingNodeModulesPlugin = require('react-dev-utils/WatchMissingNodeM
const ModuleScopePlugin = require('react-dev-utils/ModuleScopePlugin');
const getCSSModuleLocalIdent = require('react-dev-utils/getCSSModuleLocalIdent');
const paths = require('./paths');
const modules = require('./modules');
rovansteen marked this conversation as resolved.
Show resolved Hide resolved
const getClientEnvironment = require('./env');
const ModuleNotFoundPlugin = require('react-dev-utils/ModuleNotFoundPlugin');
const ForkTsCheckerWebpackPlugin = require('react-dev-utils/ForkTsCheckerWebpackPlugin');
Expand Down Expand Up @@ -259,8 +260,7 @@ module.exports = function(webpackEnv) {
// if there are any conflicts. This matches Node resolution mechanism.
// https://github.com/facebook/create-react-app/issues/253
modules: ['node_modules'].concat(
// It is guaranteed to exist because we tweak it in `env.js`
process.env.NODE_PATH.split(path.delimiter).filter(Boolean)
modules.additionalModulePath ? [modules.additionalModulePath] : []
),
// These are the reasonable defaults supported by the Node ecosystem.
// We also include JSX as a common component filename extension to support
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import initDOM from './initDOM';

describe('Integration', () => {
describe('jsconfig.json/tsconfig.json', () => {
it('Supports setting baseUrl to src', async () => {
const doc = await initDOM('base-url');

expect(doc.getElementById('feature-base-url').childElementCount).toBe(4);
doc.defaultView.close();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,6 @@ describe('Integration', () => {
doc.defaultView.close();
});

it('NODE_PATH', async () => {
iansu marked this conversation as resolved.
Show resolved Hide resolved
const doc = await initDOM('node-path');

expect(doc.getElementById('feature-node-path').childElementCount).toBe(4);
doc.defaultView.close();
});

it('PUBLIC_URL', async () => {
const doc = await initDOM('public-url');

Expand Down
5 changes: 5 additions & 0 deletions packages/react-scripts/fixtures/kitchensink/jsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"compilerOptions": {
"baseUrl": "src"
mrmckeb marked this conversation as resolved.
Show resolved Hide resolved
}
}
8 changes: 5 additions & 3 deletions packages/react-scripts/fixtures/kitchensink/src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,6 @@ class App extends Component {
this.setFeature(f.default)
);
break;
case 'node-path':
iansu marked this conversation as resolved.
Show resolved Hide resolved
import('./features/env/NodePath').then(f => this.setFeature(f.default));
break;
case 'no-ext-inclusion':
import('./features/webpack/NoExtInclusion').then(f =>
this.setFeature(f.default)
Expand Down Expand Up @@ -222,6 +219,11 @@ class App extends Component {
this.setFeature(f.default)
);
break;
case 'base-url':
import('./features/config/BaseUrl').then(f =>
this.setFeature(f.default)
);
break;
default:
throw new Error(`Missing feature "${feature}"`);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export default class extends Component {

render() {
return (
<div id="feature-node-path">
<div id="feature-base-url">
{this.state.users.map(user => (
<div key={user.id}>{user.name}</div>
))}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@

import React from 'react';
import ReactDOM from 'react-dom';
import NodePath from './NodePath';
import NodePath from './BaseUrl';

describe('NODE_PATH', () => {
describe('BASE_URL', () => {
it('renders without crashing', () => {
const div = document.createElement('div');
return new Promise(resolve => {
Expand Down
9 changes: 9 additions & 0 deletions packages/react-scripts/scripts/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,15 @@ if (process.env.HOST) {
console.log();
}

// We used to support resolving modules according to `NODE_PATH`.
// This now has been deprecated in favor of jsconfig/tsconfig.json
// This lets you use absolute paths in imports inside large monorepos:
if (process.env.NODE_PATH) {
console.log(
'Setting NODE_PATH to resolve modules absolutely has been deprecated in favor of setting baseUrl in jsconfig.json (or tsconfig.json if you are using TypeScript).'
);
}

// We require that you explictly set browsers and do not fall back to
// browserslist defaults.
const { checkBrowsers } = require('react-dev-utils/browsersHelper');
Expand Down
4 changes: 4 additions & 0 deletions packages/react-scripts/scripts/utils/createJestConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
const fs = require('fs');
const chalk = require('react-dev-utils/chalk');
const paths = require('../../config/paths');
const modules = require('../../config/modules');

module.exports = (resolve, rootDir, isEjecting) => {
// Use this instead of `paths.testsSetup` to avoid putting
Expand Down Expand Up @@ -50,6 +51,9 @@ module.exports = (resolve, rootDir, isEjecting) => {
'[/\\\\]node_modules[/\\\\].+\\.(js|jsx|ts|tsx)$',
'^.+\\.module\\.(css|sass|scss)$',
],
modulePaths: modules.additionalModulePath
? [modules.additionalModulePath]
: [],
moduleNameMapper: {
'^react-native$': 'react-native-web',
'^.+\\.module\\.(css|sass|scss)$': 'identity-obj-proxy',
Expand Down
11 changes: 4 additions & 7 deletions packages/react-scripts/scripts/utils/verifyTypeScriptSetup.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ const immer = require('react-dev-utils/immer').produce;
const globby = require('react-dev-utils/globby').sync;

function writeJson(fileName, object) {
fs.writeFileSync(fileName, JSON.stringify(object, null, 2).replace(/\n/g, os.EOL) + os.EOL);
fs.writeFileSync(
fileName,
JSON.stringify(object, null, 2).replace(/\n/g, os.EOL) + os.EOL
);
}

function verifyNoTypeScript() {
Expand Down Expand Up @@ -124,12 +127,6 @@ function verifyTypeScriptSetup() {
value: 'preserve',
reason: 'JSX is compiled by Babel',
},
// We do not support absolute imports, though this may come as a future
// enhancement
baseUrl: {
value: undefined,
reason: 'absolute imports are not supported (yet)',
},
paths: { value: undefined, reason: 'aliased imports are not supported' },
};

Expand Down
2 changes: 0 additions & 2 deletions tasks/e2e-kitchensink.sh
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ npm link "$temp_module_path/node_modules/test-integrity"

# Test the build
REACT_APP_SHELL_ENV_MESSAGE=fromtheshell \
NODE_PATH=src \
PUBLIC_URL=http://www.example.org/spa/ \
yarn build

Expand All @@ -134,7 +133,6 @@ exists build/static/js/main.*.js
# https://facebook.github.io/jest/docs/en/troubleshooting.html#tests-are-extremely-slow-on-docker-and-or-continuous-integration-ci-server
REACT_APP_SHELL_ENV_MESSAGE=fromtheshell \
CI=true \
NODE_PATH=src \
NODE_ENV=test \
yarn test --no-cache --runInBand --testPathPattern=src

Expand Down
10 changes: 10 additions & 0 deletions test/fixtures/typescript/src/App.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,13 @@ it('supports decorators', () => {
const app = new App();
expect(app.decorated).toBe(42);
});

it('supports loading modules with baseUrl', () => {
const app = new App();
expect(app.users).toEqual([
{ id: 1, name: '1' },
{ id: 2, name: '2' },
{ id: 3, name: '3' },
{ id: 4, name: '4' },
]);
});
3 changes: 3 additions & 0 deletions test/fixtures/typescript/src/App.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import absoluteLoad from 'absoluteLoad';

interface MyType {
foo: number;
bar: boolean;
Expand All @@ -12,6 +14,7 @@ class App {
n = App.foo.baz!.n;
@propertyDecorator
decorated = 5;
users = absoluteLoad();
}

function annotation(target: any) {
Expand Down
13 changes: 13 additions & 0 deletions test/fixtures/typescript/src/absoluteLoad.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

export default () => [
{ id: 1, name: '1' },
{ id: 2, name: '2' },
{ id: 3, name: '3' },
{ id: 4, name: '4' },
];
1 change: 1 addition & 0 deletions test/fixtures/typescript/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"compilerOptions": {
"baseUrl": "src",
rovansteen marked this conversation as resolved.
Show resolved Hide resolved
"experimentalDecorators": true
}
}