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 support for PUBLIC_URL env variable #937

Closed
wants to merge 5 commits into from
Closed
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
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) {
Copy link

Choose a reason for hiding this comment

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

variable naming and flow in this method can be improved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you please specify what do you mean?

Copy link

Choose a reason for hiding this comment

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

sorry for the tersity.

the middle line could be irrelevant depending on envPublicUrl, which comes as a surprise while reading. I think you should branch earlier on if (envPublicUrl) to make that clear.

There's also confusion between "public url", "homepage path", and "served path". It seems like this method is saying that the served path is derived from the public url. So I don't think "homepage" should be in the variable names.

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 admit that is convoluted, thanks for the suggestion!

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')),
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) {
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);
// 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);
Copy link

Choose a reason for hiding this comment

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

similar to how you renamed var homepagePath to var publicUrl in build.js to match the variable from paths that it was being set from, I think you should rename this one to var servedPath

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In build.js I renamed it because homepagePath wasn't significative any longer, while in this case I'm naming it to match the usage, not where it comes from.

// Webpack uses publicPath to determine where the app is being served from.

I recognize this is not the happiest choice of names..

// 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
9 changes: 8 additions & 1 deletion packages/react-scripts/fixtures/kitchensink/src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ class App extends React.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 @@ -87,6 +88,9 @@ class App extends React.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 @@ -106,6 +110,9 @@ class App extends React.Component {
require.ensure([], () => this.setFeature(require('./features/webpack/UnknownExtInclusion').default));
break;
default:
if (feature) {
throw new Error(`Missing feature "${feature}"`);
}
this.setFeature(null);
break;
}
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);
});
});
24 changes: 13 additions & 11 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('You can control this with the ' + chalk.green('homepage') + ' field in your ' + chalk.cyan('package.json') + '.');
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') + ', or with the ' + chalk.green('PUBLIC_URL') + ' environment variable.');
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not even sure we should keep ""homepage" key in package.json"-approach working. It is confusing that package.json contains deploy specific info, don't you think?

Copy link
Contributor Author

@EnoahNetzach EnoahNetzach Nov 3, 2016

Choose a reason for hiding this comment

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

Were it for me, I'd drop it entirely.
I think also that the double behavior ("homepage" -> pathname vs PUBLIC_URL -> URL) could be misleading.
If you want the pathname only, give the pathname only.

The only downside I see is if someone were to publish to gh-pages, and inserted the pathname only. The good explanation about how to publish it is lost..

Copy link

Choose a reason for hiding this comment

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

I realize this is an older post but this might help for publishing to gh-pages with a set homepage: https://itnext.io/so-you-want-to-host-your-single-age-react-app-on-github-pages-a826ab01e48

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 All @@ -193,20 +195,20 @@ function build(previousSizeMap) {
} else if (publicPath !== '/') {
// "homepage": "http://mywebsite.com/project"
console.log('The project was built assuming it is hosted at ' + chalk.green(publicPath) + '.');
console.log('You can control this with the ' + chalk.green('homepage') + ' field in your ' + chalk.cyan('package.json') + '.');
console.log('You can control this with the ' + chalk.green('homepage') + ' field in your ' + chalk.cyan('package.json') + ', or with the ' + chalk.green('PUBLIC_URL') + ' environment variable.');
console.log();
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('You can control this with the ' + chalk.green('homepage') + ' field in your ' + chalk.cyan('package.json') + '.');
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') + ', or with the ' + chalk.green('PUBLIC_URL') + ' environment variable.');
console.log();
} else {
// no homepage
console.log('To override this, specify the ' + chalk.green('homepage') + ' in your ' + chalk.cyan('package.json') + '.');
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') + ', or with the ' + chalk.green('PUBLIC_URL') + ' environment variable.');
console.log('For example, add this to build it for GitHub Pages:')
console.log();
console.log(' ' + chalk.green('"homepage"') + chalk.cyan(': ') + chalk.green('"http://myname.github.io/myapp"') + chalk.cyan(','));
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