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.

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

Correctly lookup assets when using a relative build directory #5163

Merged
merged 4 commits into from
Sep 29, 2018
Merged

Correctly lookup assets when using a relative build directory #5163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c39b51e
Correctly lookup assets when using a relative build directory
Timer Sep 28, 2018
06a1338
Hah, var ...
Timer Sep 28, 2018
136096f
Add e2e test for relative paths
Timer Sep 29, 2018
7973c81
Format svg
Timer Sep 29, 2018
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
37 changes: 37 additions & 0 deletions fixtures/smoke/relative-paths/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
const fs = require('fs-extra');
const globby = require('globby');
const path = require('path');
const {
bootstrap,
isSuccessfulDevelopment,
isSuccessfulProduction,
} = require('../../utils');
beforeEach(async () => {
await bootstrap({ directory: global.testDirectory, template: __dirname });
});

describe('relative paths', () => {
// TODO: enable when development relative paths are supported
xit('builds in development', async () => {
await isSuccessfulDevelopment({ directory: global.testDirectory });
});
it('builds in production', async () => {
await isSuccessfulProduction({ directory: global.testDirectory });

const buildDir = path.join(global.testDirectory, 'build');
const cssFile = path.join(
buildDir,
globby.sync('**/*.css', { cwd: buildDir }).pop()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This future proofs our file structure.

);
const svgFile = path.join(
buildDir,
globby.sync('**/*.svg', { cwd: buildDir }).pop()
);
const desiredPath = /url\((.+?)\)/
.exec(fs.readFileSync(cssFile, 'utf8'))
.pop();
expect(path.resolve(path.join(path.dirname(cssFile), desiredPath))).toBe(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolves it as the browser would.

path.resolve(svgFile)
);
});
});
6 changes: 6 additions & 0 deletions fixtures/smoke/relative-paths/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"dependencies": {
"react-scripts": "latest"
},
"homepage": "."
}
9 changes: 9 additions & 0 deletions fixtures/smoke/relative-paths/public/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!DOCTYPE html>
<html lang="en">
<head>
<title>React App</title>
</head>
<body>
<div id="root"></div>
</body>
</html>
8 changes: 8 additions & 0 deletions fixtures/smoke/relative-paths/src/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
.RootSvg:before {
display: block;
content: ' ';
background-image: url(./logo.svg);
background-size: 28px 28px;
height: 28px;
width: 28px;
}
1 change: 1 addition & 0 deletions fixtures/smoke/relative-paths/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './index.css';
8 changes: 8 additions & 0 deletions fixtures/smoke/relative-paths/src/logo.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"execa": "1.0.0",
"fs-extra": "^7.0.0",
"get-port": "^4.0.0",
"globby": "^8.0.1",
"husky": "1.0.0-rc.15",
"jest": "^23.6.0",
"lerna": "2.9.1",
Expand Down
11 changes: 10 additions & 1 deletion packages/react-scripts/config/webpack.config.prod.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ const getCacheIdentifier = require('react-dev-utils/getCacheIdentifier');
// Webpack uses `publicPath` to determine where the app is being served from.
// It requires a trailing slash, or the file assets will get an incorrect path.
const publicPath = paths.servedPath;
// Some apps do not use client-side routing with pushState.
// For these, "homepage" can be set to "." to enable relative asset paths.
const shouldUseRelativeAssetPaths = publicPath === './';
// Source maps are resource heavy and can cause out of memory issue for large source files.
const shouldUseSourceMap = process.env.GENERATE_SOURCEMAP !== 'false';
// `publicUrl` is just like `publicPath`, but we will provide it to our app
Expand All @@ -52,7 +55,13 @@ const sassModuleRegex = /\.module\.(scss|sass)$/;
// common function to get style loaders
const getStyleLoaders = (cssOptions, preProcessor) => {
const loaders = [
MiniCssExtractPlugin.loader,
{
loader: MiniCssExtractPlugin.loader,
options: Object.assign(
{},
shouldUseRelativeAssetPaths ? { publicPath: '../../' } : undefined
),
},
{
loader: require.resolve('css-loader'),
options: cssOptions,
Expand Down