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 all commits
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
44 changes: 41 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,37 @@ var nodePaths = (process.env.NODE_PATH || '')
.filter(folder => !path.isAbsolute(folder))
.map(resolveApp);

var envPublicUrl = process.env.PUBLIC_URL;

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

Choose a reason for hiding this comment

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

Shouldn't this have been path.length?

Copy link
Contributor

@wchargin wchargin May 8, 2018

Choose a reason for hiding this comment

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

Agreed that this is wrong, though I think that it should be 0, not path.length. This code works very accidentally, because Number(path) is NaN, which is converted to +0 by ToInteger in the definition of String.prototype.substr:

(edited to include links to docs)

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed in #4424.

} else if (!hasSlash && needsSlash) {
return path + '/';
} else {
return path;
}
}

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);
var servedUrl = envPublicUrl || (
publicUrl ? url.parse(publicUrl).pathname : '/'
);
return ensureSlash(servedUrl, true);
}

// config after eject: we're in ./config/
module.exports = {
appBuild: resolveApp('build'),
Expand All @@ -52,7 +84,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 +107,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 +125,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
2 changes: 1 addition & 1 deletion packages/react-scripts/config/webpack.config.dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ module.exports = {
// ** ADDING/UPDATING LOADERS **
// The "url" loader handles all assets unless explicitly excluded.
// The `exclude` list *must* be updated with every change to loader extensions.
// When adding a new loader, you must add its `test`
// When adding a new loader, you must add its `test`
// as a new entry in the `exclude` list for "url" loader.

// "url" loader embeds assets smaller than specified size as data URLs to avoid requests.
Expand Down
26 changes: 4 additions & 22 deletions packages/react-scripts/config/webpack.config.prod.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,31 +24,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 = paths.servedPath;
// `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 = publicPath.slice(0, -1);
// Get environment variables to inject into our app.
var env = getClientEnvironment(publicUrl);

Expand Down Expand Up @@ -123,7 +105,7 @@ module.exports = {
// ** ADDING/UPDATING LOADERS **
// The "url" loader handles all assets unless explicitly excluded.
// The `exclude` list *must* be updated with every change to loader extensions.
// When adding a new loader, you must add its `test`
// When adding a new loader, you must add its `test`
// as a new entry in the `exclude` list in the "url" loader.

// "url" loader embeds assets smaller than specified size as data URLs to avoid requests.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,31 @@ 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.')
const prefix = process.env.NODE_ENV === 'development' ? '' : 'http://www.example.org/spa';
expect(doc.getElementById('feature-public-url').textContent).to.equal(`${prefix}.`)
expect(doc.querySelector('head link[rel="shortcut icon"]').getAttribute('href'))
.to.equal(`${prefix}/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
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
14 changes: 12 additions & 2 deletions tasks/e2e-kitchensink.sh
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,11 @@ cd test-kitchensink
npm link $root_path/packages/babel-preset-react-app

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

# Check for expected output
test -e build/*.html
test -e build/static/js/main.*.js
Expand Down Expand Up @@ -144,6 +148,7 @@ E2E_FILE=./build/index.html \
CI=true \
NODE_PATH=src \
NODE_ENV=production \
PUBLIC_URL=http://www.example.org/spa/ \
node_modules/.bin/mocha --require babel-register --require babel-polyfill integration/*.test.js

# ******************************************************************************
Expand All @@ -166,7 +171,11 @@ npm link $root_path/packages/react-scripts
rm .babelrc

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

# Check for expected output
test -e build/*.html
test -e build/static/js/main.*.js
Expand Down Expand Up @@ -196,6 +205,7 @@ E2E_FILE=./build/index.html \
CI=true \
NODE_ENV=production \
NODE_PATH=src \
PUBLIC_URL=http://www.example.org/spa/ \
node_modules/.bin/mocha --require babel-register --require babel-polyfill integration/*.test.js

# Cleanup
Expand Down