-
-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'); | ||
|
||
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similar to how you renamed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In build.js I renamed it because
I recognize this is not the happiest choice of names.. |
||
// Get environment variables to inject into our app. | ||
var env = getClientEnvironment(publicUrl); | ||
|
||
|
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); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'); | ||
|
@@ -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.'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not even sure we should keep " There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Were it for me, I'd drop it entirely. The only downside I see is if someone were to publish to There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
|
@@ -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(',')); | ||
|
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 onif (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.
There was a problem hiding this comment.
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!