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

Add PUBLIC_URL env variable for advanced use (#937) #1504

Merged
merged 2 commits into from
Feb 10, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
35 changes: 32 additions & 3 deletions packages/react-scripts/config/paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

var path = require('path');
var fs = require('fs');
var url = require('url');

// Make sure any symlinks in the project folder are resolved:
// https://github.com/facebookincubator/create-react-app/issues/637
Expand Down Expand Up @@ -40,6 +41,28 @@ var nodePaths = (process.env.NODE_PATH || '')
.filter(folder => !path.isAbsolute(folder))
.map(resolveApp);

var envPublicUrl = process.env.PUBLIC_URL;

function getPublicUrl(appPackageJson) {
return envPublicUrl || require(appPackageJson).homepage;
}

// We use `PUBLIC_URL` environment variable or "homepage" field to infer
// "public path" at which the app is served.
// Webpack needs to know it to put the right <script> hrefs into HTML even in
// single-page apps that may serve index.html for nested URLs like /todos/42.
// We can't use a relative path in HTML because we don't want to load something
// like /todos/42/static/js/bundle.7289d.js. We have to know the root.
function getServedPath(appPackageJson) {
var publicUrl = getPublicUrl(appPackageJson);
if (!publicUrl) {
return '/';
} else if (envPublicUrl) {
return publicUrl;
}
return url.parse(publicUrl).pathname;
}

// config after eject: we're in ./config/
module.exports = {
appBuild: resolveApp('build'),
Expand All @@ -52,7 +75,9 @@ module.exports = {
testsSetup: resolveApp('src/setupTests.js'),
appNodeModules: resolveApp('node_modules'),
ownNodeModules: resolveApp('node_modules'),
nodePaths: nodePaths
nodePaths: nodePaths,
publicUrl: getPublicUrl(resolveApp('package.json')),
Copy link
Contributor

Choose a reason for hiding this comment

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

It's fine to give these more verbose names if that would make it easier to understand what they represent. Right now it's not super obvious to me what the difference is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went ahead and just removed the third variable as it was unnecessary. publicUrl and servedPath seem straightforward to me (now that servedUrl is gone).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you prefer different names still?

servedPath: getServedPath(resolveApp('package.json'))
};

// @remove-on-eject-begin
Expand All @@ -73,7 +98,9 @@ module.exports = {
appNodeModules: resolveApp('node_modules'),
// this is empty with npm3 but node resolution searches higher anyway:
ownNodeModules: resolveOwn('../node_modules'),
nodePaths: nodePaths
nodePaths: nodePaths,
publicUrl: getPublicUrl(resolveApp('package.json')),
servedPath: getServedPath(resolveApp('package.json'))
};

// config before publish: we're in ./packages/react-scripts/config/
Expand All @@ -89,7 +116,9 @@ if (__dirname.indexOf(path.join('packages', 'react-scripts', 'config')) !== -1)
testsSetup: resolveOwn('../template/src/setupTests.js'),
appNodeModules: resolveOwn('../node_modules'),
ownNodeModules: resolveOwn('../node_modules'),
nodePaths: nodePaths
nodePaths: nodePaths,
publicUrl: getPublicUrl(resolveOwn('../package.json')),
servedPath: getServedPath(resolveOwn('../package.json'))
};
}
// @remove-on-eject-end
10 changes: 10 additions & 0 deletions packages/react-scripts/config/utils/ensureSlash.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module.exports = function ensureSlash(path, needsSlash) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would like to move this into paths.js and instead export publicPath, publicUrl and whatever else is needed from it. This would be more explicit IMO.

Copy link
Contributor

Choose a reason for hiding this comment

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

So ideally Webpack configs shouldn't need ensureSlash because they should already have everything they need in the final form in the paths.

var hasSlash = path.endsWith('/');
if (hasSlash && !needsSlash) {
return path.substr(path, path.length - 1);
} else if (!hasSlash && needsSlash) {
return path + '/';
} else {
return path;
}
}
3 changes: 2 additions & 1 deletion packages/react-scripts/config/webpack.config.dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ var HtmlWebpackPlugin = require('html-webpack-plugin');
var CaseSensitivePathsPlugin = require('case-sensitive-paths-webpack-plugin');
var InterpolateHtmlPlugin = require('react-dev-utils/InterpolateHtmlPlugin');
var WatchMissingNodeModulesPlugin = require('react-dev-utils/WatchMissingNodeModulesPlugin');
var ensureSlash = require('./utils/ensureSlash');
var getClientEnvironment = require('./env');
var paths = require('./paths');

Expand All @@ -29,7 +30,7 @@ var publicPath = '/';
// `publicUrl` is just like `publicPath`, but we will provide it to our app
// as %PUBLIC_URL% in `index.html` and `process.env.PUBLIC_URL` in JavaScript.
// Omit trailing slash as %PUBLIC_PATH%/xyz looks better than %PUBLIC_PATH%xyz.
var publicUrl = '';
var publicUrl = ensureSlash(paths.servedPath, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

I expect that this might break HMR when PUBLIC_URL is specified. I think we might be assuming we're serving from the root somewhere either in start.js or in webpack hot client.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not really sure why this was changed, but I know it still worked after. I didn't test HMR, though -- I'll remove it as I don't know why the homepage is relevant to the dev environment.

// Get environment variables to inject into our app.
var env = getClientEnvironment(publicUrl);

Expand Down
25 changes: 4 additions & 21 deletions packages/react-scripts/config/webpack.config.prod.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ var ExtractTextPlugin = require('extract-text-webpack-plugin');
var ManifestPlugin = require('webpack-manifest-plugin');
var InterpolateHtmlPlugin = require('react-dev-utils/InterpolateHtmlPlugin');
var url = require('url');
var ensureSlash = require('./utils/ensureSlash');
var paths = require('./paths');
var getClientEnvironment = require('./env');

Expand All @@ -24,31 +25,13 @@ var getClientEnvironment = require('./env');
var path = require('path');
// @remove-on-eject-end

function ensureSlash(path, needsSlash) {
var hasSlash = path.endsWith('/');
if (hasSlash && !needsSlash) {
return path.substr(path, path.length - 1);
} else if (!hasSlash && needsSlash) {
return path + '/';
} else {
return path;
}
}

// We use "homepage" field to infer "public path" at which the app is served.
// Webpack needs to know it to put the right <script> hrefs into HTML even in
// single-page apps that may serve index.html for nested URLs like /todos/42.
// We can't use a relative path in HTML because we don't want to load something
// like /todos/42/static/js/bundle.7289d.js. We have to know the root.
var homepagePath = require(paths.appPackageJson).homepage;
var homepagePathname = homepagePath ? url.parse(homepagePath).pathname : '/';
// 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.
var publicPath = ensureSlash(homepagePathname, true);
var publicPath = ensureSlash(paths.servedPath, true);
// `publicUrl` is just like `publicPath`, but we will provide it to our app
// as %PUBLIC_URL% in `index.html` and `process.env.PUBLIC_URL` in JavaScript.
// Omit trailing slash as %PUBLIC_PATH%/xyz looks better than %PUBLIC_PATH%xyz.
var publicUrl = ensureSlash(homepagePathname, false);
// Omit trailing slash as %PUBLIC_URL%/xyz looks better than %PUBLIC_URL%xyz.
var publicUrl = ensureSlash(paths.servedPath, false);
// Get environment variables to inject into our app.
var env = getClientEnvironment(publicUrl);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,30 @@ import initDOM from './initDOM'

describe('Integration', () => {
describe('Environment variables', () => {
it('file env variables', async () => {
const doc = await initDOM('file-env-variables')

expect(doc.getElementById('feature-file-env-variables').textContent).to.equal('fromtheenvfile.')
})

it('NODE_PATH', async () => {
const doc = await initDOM('node-path')

expect(doc.getElementById('feature-node-path').childElementCount).to.equal(4)
})

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

expect(doc.getElementById('feature-shell-env-variables').textContent).to.equal('fromtheshell.')
expect(doc.getElementById('feature-public-url').textContent).to.equal('http://www.example.org/spa.')
expect(doc.querySelector('head link[rel="shortcut icon"]').getAttribute('href'))
.to.equal('http://www.example.org/spa/favicon.ico')
})

it('file env variables', async () => {
const doc = await initDOM('file-env-variables')
it('shell env variables', async () => {
const doc = await initDOM('shell-env-variables')

expect(doc.getElementById('feature-file-env-variables').textContent).to.equal('fromtheenvfile.')
expect(doc.getElementById('feature-shell-env-variables').textContent).to.equal('fromtheshell.')
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ if (process.env.E2E_FILE) {
const markup = fs.readFileSync(file, 'utf8')
getMarkup = () => markup

const pathPrefix = process.env.PUBLIC_URL.replace(/^https?:\/\/[^\/]+\/?/, '')

resourceLoader = (resource, callback) => callback(
null,
fs.readFileSync(path.join(path.dirname(file), resource.url.pathname), 'utf8')
fs.readFileSync(path.join(path.dirname(file), resource.url.pathname.replace(pathPrefix, '')), 'utf8')
)
} else if (process.env.E2E_URL) {
getMarkup = () => new Promise(resolve => {
Expand All @@ -37,7 +39,7 @@ if (process.env.E2E_FILE) {

export default feature => new Promise(async resolve => {
const markup = await getMarkup()
const host = process.env.E2E_URL || 'http://localhost:3000'
const host = process.env.E2E_URL || 'http://www.example.org/spa:3000'
const doc = jsdom.jsdom(markup, {
features: {
FetchExternalResources: ['script', 'css'],
Expand Down
10 changes: 7 additions & 3 deletions packages/react-scripts/fixtures/kitchensink/src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class BuiltEmitter extends Component {
}

componentDidMount() {
const { feature } = this.props
const { feature } = this.props;

// Class components must call this.props.onReady when they're ready for the test.
// We will assume functional components are ready immediately after mounting.
Expand Down Expand Up @@ -44,7 +44,8 @@ class App extends Component {
}

componentDidMount() {
switch (location.hash.slice(1)) {
const feature = location.hash.slice(1);
switch (feature) {
case 'array-destructuring':
require.ensure([], () => this.setFeature(require('./features/syntax/ArrayDestructuring').default));
break;
Expand Down Expand Up @@ -99,6 +100,9 @@ class App extends Component {
case 'promises':
require.ensure([], () => this.setFeature(require('./features/syntax/Promises').default));
break;
case 'public-url':
require.ensure([], () => this.setFeature(require('./features/env/PublicUrl').default));
break;
case 'rest-and-default':
require.ensure([], () => this.setFeature(require('./features/syntax/RestAndDefault').default));
break;
Expand All @@ -117,7 +121,7 @@ class App extends Component {
case 'unknown-ext-inclusion':
require.ensure([], () => this.setFeature(require('./features/webpack/UnknownExtInclusion').default));
break;
default: throw new Error('Unknown feature!');
default: throw new Error(`Missing feature "${feature}"`);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import React from 'react'

export default () => (
<span id="feature-public-url">{process.env.PUBLIC_URL}.</span>
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import React from 'react';
import ReactDOM from 'react-dom';
import PublicUrl from './PublicUrl';

describe('PUBLIC_URL', () => {
it('renders without crashing', () => {
const div = document.createElement('div');
ReactDOM.render(<PublicUrl />, div);
});
});
16 changes: 9 additions & 7 deletions packages/react-scripts/scripts/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ require('dotenv').config({silent: true});
var chalk = require('chalk');
var fs = require('fs-extra');
var path = require('path');
var url = require('url');
var filesize = require('filesize');
var gzipSize = require('gzip-size').sync;
var webpack = require('webpack');
Expand Down Expand Up @@ -158,15 +159,16 @@ function build(previousSizeMap) {

var openCommand = process.platform === 'win32' ? 'start' : 'open';
var appPackage = require(paths.appPackageJson);
var homepagePath = appPackage.homepage;
var publicUrl = paths.publicUrl;
var publicPath = config.output.publicPath;
if (homepagePath && homepagePath.indexOf('.github.io/') !== -1) {
var publicPathname = url.parse(publicPath).pathname;
if (publicUrl && publicUrl.indexOf('.github.io/') !== -1) {
// "homepage": "http://user.github.io/project"
console.log('The project was built assuming it is hosted at ' + chalk.green(publicPath) + '.');
console.log('The project was built assuming it is hosted at ' + chalk.green(publicPathname) + '.');
console.log('You can control this with the ' + chalk.green('homepage') + ' field in your ' + chalk.cyan('package.json') + '.');
console.log();
console.log('The ' + chalk.cyan('build') + ' folder is ready to be deployed.');
console.log('To publish it at ' + chalk.green(homepagePath) + ', run:');
console.log('To publish it at ' + chalk.green(publicUrl) + ', run:');
// If script deploy has been added to package.json, skip the instructions
if (typeof appPackage.scripts.deploy === 'undefined') {
console.log();
Expand Down Expand Up @@ -198,14 +200,14 @@ function build(previousSizeMap) {
console.log('The ' + chalk.cyan('build') + ' folder is ready to be deployed.');
console.log();
} else {
// no homepage or "homepage": "http://mywebsite.com"
console.log('The project was built assuming it is hosted at the server root.');
if (homepagePath) {
if (publicUrl) {
// "homepage": "http://mywebsite.com"
console.log('The project was built assuming it is hosted at ' + chalk.green(publicUrl) + '.');
console.log('You can control this with the ' + chalk.green('homepage') + ' field in your ' + chalk.cyan('package.json') + '.');
console.log();
} else {
// no homepage
console.log('The project was built assuming it is hosted at the server root.');
console.log('To override this, specify the ' + chalk.green('homepage') + ' in your ' + chalk.cyan('package.json') + '.');
console.log('For example, add this to build it for GitHub Pages:')
console.log();
Expand Down
17 changes: 10 additions & 7 deletions packages/react-scripts/scripts/eject.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,6 @@ prompt(
}
}

var folders = [
'config',
path.join('config', 'jest'),
'scripts'
];

var files = [
path.join('config', 'env.js'),
path.join('config', 'paths.js'),
Expand All @@ -57,11 +51,20 @@ prompt(
path.join('config', 'webpack.config.prod.js'),
path.join('config', 'jest', 'cssTransform.js'),
path.join('config', 'jest', 'fileTransform.js'),
path.join('config', 'utils', 'ensureSlash.js'),
path.join('scripts', 'build.js'),
path.join('scripts', 'start.js'),
path.join('scripts', 'test.js')
path.join('scripts', 'test.js'),
];

var folders = files.reduce(function(prevFolders, file) {
var dirname = path.dirname(file);
if (prevFolders.indexOf(dirname) === -1) {
return prevFolders.concat(dirname);
}
return prevFolders;
}, []);

// Ensure that the app folder is clean and we won't override any files
folders.forEach(verifyAbsent);
files.forEach(verifyAbsent);
Expand Down
2 changes: 1 addition & 1 deletion packages/react-scripts/scripts/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ function runDevServer(host, port, protocol) {
// project directory is dangerous because we may expose sensitive files.
// Instead, we establish a convention that only files in `public` directory
// get served. Our build script will copy `public` into the `build` folder.
// In `index.html`, you can get URL of `public` folder with %PUBLIC_PATH%:
// In `index.html`, you can get URL of `public` folder with %PUBLIC_URL%:
// <link rel="shortcut icon" href="%PUBLIC_URL%/favicon.ico">
// In JavaScript code, you can access it with `process.env.PUBLIC_URL`.
// Note that we only recommend to use `public` folder as an escape hatch
Expand Down
Loading