Skip to content
This repository has been archived by the owner on Apr 24, 2024. It is now read-only.

react-scripts > v2.1.1 don't have a webpack.config.dev #20

Merged
merged 6 commits into from
Jan 10, 2019
Merged

react-scripts > v2.1.1 don't have a webpack.config.dev #20

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a1d0e24
Merge pull request #16 from Nargonath/develop
Nargonath Nov 18, 2018
d722abb
react-scripts > v2.1.1 don't have a webpack.config.dev
looseend Jan 7, 2019
0c504fc
make isEjected workk of config/paths.js
looseend Jan 7, 2019
1d56f1c
Back out get version changes
looseend Jan 10, 2019
bfbe83a
Back out get version changes
looseend Jan 10, 2019
74410d5
Back out get version changes
looseend Jan 10, 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
19 changes: 14 additions & 5 deletions scripts/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,22 @@ const {
flags: { buildPath, publicPath, reactScriptsVersion, verbose },
} = require('../utils/cliHandler');
const { getReactScriptsVersion, isEjected } = require('../utils');

const { major, minor, patch } = getReactScriptsVersion(reactScriptsVersion);

const paths = isEjected ? importCwd('./config/paths') : importCwd('react-scripts/config/paths');
const webpack = importCwd('webpack');
const config = isEjected
? importCwd('./config/webpack.config.dev')
: importCwd('react-scripts/config/webpack.config.dev');

const config = (major >= 2 && minor >= 1 && patch >= 2) ?
(isEjected
? importCwd('./config/webpack.config')
Nargonath marked this conversation as resolved.
Show resolved Hide resolved
: importCwd('react-scripts/config/webpack.config'))('development')
:
isEjected
? importCwd('./config/webpack.config.dev')
: importCwd('react-scripts/config/webpack.config.dev');


const HtmlWebpackPlugin = importCwd('html-webpack-plugin');
const InterpolateHtmlPlugin = importCwd('react-dev-utils/InterpolateHtmlPlugin');
const getClientEnvironment = isEjected
Expand Down Expand Up @@ -50,8 +61,6 @@ config.output.filename = `js/bundle.js`;
config.output.chunkFilename = `js/[name].chunk.js`;

// update media path destination
const { major, minor, patch } = getReactScriptsVersion(reactScriptsVersion);

if (major >= 2) {
const oneOfIndex = minor >= 1 || patch >= 4 ? 2 : 3;
config.module.rules[oneOfIndex].oneOf[0].options.name = `media/[name].[hash:8].[ext]`;
Expand Down
9 changes: 5 additions & 4 deletions utils/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const DEFAULT_VERSION = {
patch: 4,
};

exports.isEjected = fs.pathExistsSync(path.join(process.cwd(), 'config/webpack.config.dev.js'));
exports.isEjected = fs.pathExistsSync(path.join(process.cwd(), 'config/paths.js'))
Nargonath marked this conversation as resolved.
Show resolved Hide resolved

exports.getReactScriptsVersion = function getReactScriptsVersion(cliVersion) {
if (cliVersion) {
Expand All @@ -29,10 +29,11 @@ exports.getReactScriptsVersion = function getReactScriptsVersion(cliVersion) {
}

const { dependencies } = packageJson;
const reactScriptsVersionString = /^[~^]?([.0-9]+)$/.exec(dependencies['react-scripts'])[1];
const versions = {
major: Number(semver.major(dependencies['react-scripts'])),
minor: Number(semver.minor(dependencies['react-scripts'])),
patch: Number(semver.patch(dependencies['react-scripts'])),
major: Number(semver.major(reactScriptsVersionString)),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

semver.major only accepts a version string, not a ^ or ~ prefixed comparison string.

Copy link
Owner

Choose a reason for hiding this comment

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

Hum, did you get an error with the previous behavior? I'd rather prefer that the prefix is taken into account because having ^1.3.0 does not mean that you have 1.3.0 installed hence you could have 1.3.7. That's the reason I use the semver utils for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes:

TypeError: Invalid Version: ^2.1.3
at new SemVer (/Users/johnsingleton/cra-test/my-app/node_modules/semver/semver.js:312:11)

I think because "^2.1.3" is a constraint not a version number. As is the code is just pulling whatever is in package.json, so I don't think it's getting the actual installed version anyway.

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah you are right, there could always be a difference with the actual version installed but at least I feel we are a bit closer to the truth. 😄 I'll need to look a bit more in semver docs or look for another solution before I settle for this. If we don't have another solution then we'll roll with it.

Copy link
Owner

Choose a reason for hiding this comment

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

I've found a better way to get the package version and to be sure it is the actual version installed by the user. However I don't think it should be a part of this PR. I'll make the development in another PR. I'll just change your PR destination to develop and in the meantime can you delete the modifications you did to the getReactScriptsVersion function, please?

minor: Number(semver.minor(reactScriptsVersionString)),
patch: Number(semver.patch(reactScriptsVersionString)),
};
return versions;
};