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 module resolution based on baseUrl in jsconfig/tsconfig.json #6116

Closed
wants to merge 34 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
07c1f95
Set TS base url based on node_path
robertvansteen Jan 3, 2019
ec03a78
Remove TS messages for now
robertvansteen Jan 3, 2019
ebd7650
Resole baseUrl for tsconfig/jsconfig.json
robertvansteen Jan 3, 2019
4de79d5
Add baseUrl test
robertvansteen Jan 3, 2019
839a2a3
Add jsconfig.json to kitchensink
robertvansteen Jan 3, 2019
91f7794
Add BaseUrl component
robertvansteen Jan 3, 2019
be87538
Add baseUrl as modulePath for jest
robertvansteen Jan 3, 2019
e0d3ebf
Remove NODE_PATH resolving and inform user about it’s deprecation.
robertvansteen Jan 6, 2019
5d67018
Remove unused variable
robertvansteen Jan 6, 2019
bb49cf3
Remove accident typo
robertvansteen Jan 6, 2019
f5821c2
Remove unused tests & properly load baseUrl test
robertvansteen Jan 6, 2019
832cad2
Fix wrong path to BaseUrl test
robertvansteen Jan 6, 2019
401b500
Add support for aliasing @ to src
robertvansteen Jan 6, 2019
234e361
Add integration test for alias
robertvansteen Jan 6, 2019
97ea854
Log to debug tests
robertvansteen Jan 6, 2019
2aa5605
Allow setting the baseUrl to node_modules
robertvansteen Jan 6, 2019
a0ed858
Remove unnecessary prefix for Jest module paths
robertvansteen Jan 6, 2019
e52182d
Whoops.
robertvansteen Jan 6, 2019
bf5329f
Call isValidPath method to check if the baseUrl is valid
robertvansteen Jan 6, 2019
2c19adc
Use appDirectory from paths
robertvansteen Jan 6, 2019
ac8dd39
Throw error if there are both a tsconfig.json and jsconfig.json file
robertvansteen Jan 6, 2019
a9b60b1
Add better path checking & add support for different aliases for src
robertvansteen Jan 7, 2019
637e404
Rename config to modules
robertvansteen Jan 7, 2019
830415a
Fix old reference
robertvansteen Jan 7, 2019
09d9845
Update baseUrl in Typescript compiler options
robertvansteen Jan 7, 2019
db01cb9
Log modules to debug
robertvansteen Jan 7, 2019
47b7099
Correctly return prev in reducer
robertvansteen Jan 7, 2019
50e9c60
Suffix alias with a forward slash to prevent conflicts with node_modu…
robertvansteen Jan 7, 2019
229457a
Remove console.log
robertvansteen Jan 7, 2019
216eaff
Attempt to test baseUrl for Typescript
robertvansteen Jan 7, 2019
26d6bdb
Add test to verify aliases work in TypeScript as well
robertvansteen Jan 7, 2019
74ad80a
Remove baseUrl overwrite
robertvansteen Jan 7, 2019
116c9c8
Allow setting the path of an alias to a subfolder of src
robertvansteen Jan 7, 2019
ccc6b89
Work on matching jsconfig path behavior with webpack/jest
robertvansteen Jan 9, 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
22 changes: 7 additions & 15 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,21 +47,14 @@ dotenvFiles.forEach(dotenvFile => {
}
});

// We support resolving modules according to `NODE_PATH`.
// 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:
// 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);
if (process.env.NODE_PATH) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should move this check elsewhere imo -- probably in the script bin wrapper.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Timer in the bin wrapper the .env files are not parsed yet, so that fails to check the NODE_PATH properly. What's your concern with having it in this file?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, good point. And this just seems like an odd place to have the check (as a side effect of the file being evaluated).

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can just add it to start.js, omitting it from the build and test scripts.

console.log(
'Setting NODE_PATH to resolves modules absolutely has been deprecated in favor of setting baseUrl in jsconfig.json (or tsconfig.json if you are using Typescript) to achieve the same behaviour.'
);
}

// Grab NODE_ENV and REACT_APP_* environment variables and prepare them to be
// injected into the application via DefinePlugin in Webpack configuration.
Expand Down
183 changes: 183 additions & 0 deletions packages/react-scripts/config/modules.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
// @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 chalk = require('chalk');
const paths = require('./paths');

/**
* 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.'
)
);
}

/**
* Get the alias of a compilerOptions object.
*
* @param {Object} options
*/
function getAliases(options = {}) {
robertvansteen marked this conversation as resolved.
Show resolved Hide resolved
// This is an object with the alias as key
// and an array of paths as value.
// e.g. '@': ['src']
const aliases = options.paths || {};

return Object.keys(aliases).reduce(function(prev, alias) {
// Value contains the paths of the alias.
const value = aliases[alias];

// The value should be an array but we have to verify
// that because it's user input.
if (!Array.isArray(value) || value.length > 1) {
throw new Error(
chalk.red.bold(
"Your project's `alias` can only be set to an array containing `src` or a subfolder of `src`." +
' Create React App does not support other values at this time.'
)
);
}

const aliasPath = value[0];

// Alias paths are relative to the baseurl.
// If there is no baseUrl set, it will default to the root of the app.
const baseUrl = options.baseUrl
? path.resolve(paths.appPath, options.baseUrl)
: paths.appPath;
const resolvedAliasPath = path.resolve(baseUrl, aliasPath);

// We then check if the resolved alias path is src or a sub folder of src.
const relativePath = path.relative(paths.appSrc, resolvedAliasPath);
const isSrc = relativePath === '';
const isSubfolderOfSrc =
!relativePath.startsWith('../') && !relativePath.startsWith('..\\');

if (!isSrc && !isSubfolderOfSrc) {
throw new Error(
chalk.red.bold(
"Your project's `alias` can only be set to ['src'] or a subfolder of `src`." +
' Create React App does not support other values at this time.'
)
);
}

prev[alias] = resolvedAliasPath;
return prev;
}, {});
}

function getWebpackAliases(aliases) {
return Object.keys(aliases).reduce(function(prev, alias) {
let aliasPath = aliases[alias];
const endsWithWilcard = alias.endsWith('*');
// Remove trailing wildcards (/*)
alias = alias.replace(/\/?\*$/, '');
aliasPath = aliasPath.replace(/\/\*$/, '');
// Webpack aliases work a little bit different than jsconfig/tsconfig.json paths
// By default webpack aliases act as a wildcard and for an exact match you have
// to suffix it with a dollar sign.
// tsconfig/jsconfig.json work the other way around and are an exact match unless
// suffixed by a wildcard.
const webpackAlias = endsWithWilcard ? alias : alias + '$';
prev[webpackAlias] = aliasPath;
return prev;
}, {});
}

function getJestAliases(aliases) {
return Object.keys(aliases).reduce(function(prev, alias) {
const endsWithWilcard = alias.endsWith('*');
let aliasPath = aliases[alias];

alias = alias.replace(/\/?\*$/, '');
const match = endsWithWilcard ? alias + '(.*)$' : alias;

aliasPath = aliasPath.replace(/\*$/, '');
const relativeAliasPath = path.relative(paths.appPath, aliasPath);
const target = '<rootDir>/' + relativeAliasPath;

prev[match] = target + (endsWithWilcard ? '/$1' : '');
return prev;
}, {});
}

function getModules() {
// Check if TypeScript is setup
const useTypeScript = fs.existsSync(paths.appTsConfig);
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 aliases = getAliases(options);
const jestAliases = getJestAliases(aliases);
const webpackAliases = getWebpackAliases(aliases);
const additionalModulePath = getAdditionalModulePath(options);

return {
aliases: aliases,
jestAliases: jestAliases,
webpackAliases: webpackAliases,
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
5 changes: 3 additions & 2 deletions packages/react-scripts/config/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const paths = require('./paths');
const getClientEnvironment = require('./env');
const ModuleNotFoundPlugin = require('react-dev-utils/ModuleNotFoundPlugin');
const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin-alt');
const modules = require('./modules');
const typescriptFormatter = require('react-dev-utils/typescriptFormatter');
// @remove-on-eject-begin
const getCacheIdentifier = require('react-dev-utils/getCacheIdentifier');
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 All @@ -275,6 +275,7 @@ module.exports = function(webpackEnv) {
// Support React Native Web
// https://www.smashingmagazine.com/2016/08/a-glimpse-into-the-future-with-react-native-for-web/
'react-native': 'react-native-web',
...modules.webpackAliases,
},
plugins: [
// Adds support for installing with Plug'n'Play, leading to faster installs and adding
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* 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();
});

it('Supports setting @ as alias to src', async () => {
const doc = await initDOM('alias');

expect(doc.getElementById('feature-alias').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 () => {
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
8 changes: 8 additions & 0 deletions packages/react-scripts/fixtures/kitchensink/jsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"compilerOptions": {
"baseUrl": "src",
"paths": {
"@*": ["*"]
}
}
}
9 changes: 7 additions & 2 deletions packages/react-scripts/fixtures/kitchensink/src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,13 @@ class App extends Component {
this.setFeature(f.default)
);
break;
case 'node-path':
import('./features/env/NodePath').then(f => this.setFeature(f.default));
case 'base-url':
import('./features/config/BaseUrl').then(f =>
this.setFeature(f.default)
);
break;
case 'alias':
import('./features/config/Alias').then(f => this.setFeature(f.default));
break;
case 'no-ext-inclusion':
import('./features/webpack/NoExtInclusion').then(f =>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* 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 React, { Component } from 'react';
import PropTypes from 'prop-types';
import load from '@/absoluteLoad';

export default class extends Component {
static propTypes = {
onReady: PropTypes.func.isRequired,
};

constructor(props) {
super(props);
this.state = { users: [] };
}

async componentDidMount() {
const users = load();
this.setState({ users });
}

componentDidUpdate() {
this.props.onReady();
}

render() {
return (
<div id="feature-alias">
{this.state.users.map(user => (
<div key={user.id}>{user.name}</div>
))}
</div>
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@

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

describe('NODE_PATH', () => {
describe('alias', () => {
it('renders without crashing', () => {
const div = document.createElement('div');
return new Promise(resolve => {
ReactDOM.render(<NodePath onReady={resolve} />, div);
ReactDOM.render(<Alias onReady={resolve} />, div);
});
});
});
Loading