Skip to content

Commit

Permalink
Allow relative $PUBLIC_URL in dev mode
Browse files Browse the repository at this point in the history
As long as client side routing isn't used, a relative $PUBLIC_URL should
work correctly in both development and production.
There's no reason it should be limited to production.

Closes facebook#8623

Also see coder/code-server#2565
  • Loading branch information
nhooyr committed Feb 5, 2021
1 parent 3f5dea9 commit 66d3239
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 32 deletions.
37 changes: 23 additions & 14 deletions packages/react-dev-utils/__tests__/getPublicUrlOrPath.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ const tests = [
{ dev: true, homepage: '/', expect: '/' },
{ dev: true, homepage: '/test', expect: '/test/' },
{ dev: true, homepage: '/test/', expect: '/test/' },
{ dev: true, homepage: './', expect: '/' },
{ dev: true, homepage: '../', expect: '/' },
{ dev: true, homepage: '../test', expect: '/' },
{ dev: true, homepage: './test/path', expect: '/' },
{ dev: true, homepage: './', expect: './' },
{ dev: true, homepage: '../', expect: '../' },
{ dev: true, homepage: '../test', expect: '../test/' },
{ dev: true, homepage: './test/path', expect: './test/path/' },
{ dev: true, homepage: 'https://create-react-app.dev/', expect: '/' },
{
dev: true,
Expand All @@ -28,10 +28,10 @@ const tests = [
{ dev: true, publicUrl: '/', expect: '/' },
{ dev: true, publicUrl: '/test', expect: '/test/' },
{ dev: true, publicUrl: '/test/', expect: '/test/' },
{ dev: true, publicUrl: './', expect: '/' },
{ dev: true, publicUrl: '../', expect: '/' },
{ dev: true, publicUrl: '../test', expect: '/' },
{ dev: true, publicUrl: './test/path', expect: '/' },
{ dev: true, publicUrl: './', expect: './' },
{ dev: true, publicUrl: '../', expect: '../' },
{ dev: true, publicUrl: '../test', expect: '../test/' },
{ dev: true, publicUrl: './test/path', expect: './test/path/' },
{ dev: true, publicUrl: 'https://create-react-app.dev/', expect: '/' },
{
dev: true,
Expand All @@ -42,10 +42,15 @@ const tests = [
{ dev: true, publicUrl: '/', homepage: '/test', expect: '/' },
{ dev: true, publicUrl: '/test', homepage: '/path', expect: '/test/' },
{ dev: true, publicUrl: '/test/', homepage: '/test/path', expect: '/test/' },
{ dev: true, publicUrl: './', homepage: '/test', expect: '/' },
{ dev: true, publicUrl: '../', homepage: '/test', expect: '/' },
{ dev: true, publicUrl: '../test', homepage: '/test', expect: '/' },
{ dev: true, publicUrl: './test/path', homepage: '/test', expect: '/' },
{ dev: true, publicUrl: './', homepage: '/test', expect: './' },
{ dev: true, publicUrl: '../', homepage: '/test', expect: '../' },
{ dev: true, publicUrl: '../test', homepage: '/test', expect: '../test/' },
{
dev: true,
publicUrl: './test/path',
homepage: '/test',
expect: './test/path/',
},
{
dev: true,
publicUrl: 'https://create-react-app.dev/',
Expand All @@ -67,11 +72,15 @@ const tests = [
{ dev: false, homepage: '../', expect: '../' },
{ dev: false, homepage: '../test', expect: '../test/' },
{ dev: false, homepage: './test/path', expect: './test/path/' },
{ dev: false, homepage: 'https://create-react-app.dev/', expect: '/' },
{
dev: false,
homepage: 'https://create-react-app.dev/',
expect: 'https://create-react-app.dev/',
},
{
dev: false,
homepage: 'https://create-react-app.dev/test',
expect: '/test/',
expect: 'https://create-react-app.dev/test/',
},
// PRODUCTION with publicUrl
{ dev: false, publicUrl: '/', expect: '/' },
Expand Down
33 changes: 15 additions & 18 deletions packages/react-dev-utils/getPublicUrlOrPath.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ module.exports = getPublicUrlOrPath;
/**
* Returns a URL or a path with slash at the end
* In production can be URL, abolute path, relative path
* In development always will be an absolute path
* In development can be a relative or absolute path
* In development can use `path` module functions for operations
*
* @param {boolean} isEnvDevelopment
Expand All @@ -31,34 +31,31 @@ function getPublicUrlOrPath(isEnvDevelopment, homepage, envPublicUrl) {
? envPublicUrl
: envPublicUrl + '/';

// Some apps do not use client-side routing with pushState.
// For these, "$PUBLIC_URL" can be set to "." to enable relative asset paths.
if (envPublicUrl.startsWith('.')) {
return envPublicUrl;
}

// validate if `envPublicUrl` is a URL or path like
// `stubDomain` is ignored if `envPublicUrl` contains a domain
const validPublicUrl = new URL(envPublicUrl, stubDomain);

return isEnvDevelopment
? envPublicUrl.startsWith('.')
? '/'
: validPublicUrl.pathname
: // Some apps do not use client-side routing with pushState.
// For these, "homepage" can be set to "." to enable relative asset paths.
envPublicUrl;
return isEnvDevelopment ? validPublicUrl.pathname : envPublicUrl;
}

if (homepage) {
// strip last slash if exists
homepage = homepage.endsWith('/') ? homepage : homepage + '/';

// Some apps do not use client-side routing with pushState.
// For these, homepage can be set to "." to enable relative asset paths.
if (homepage.startsWith('.')) {
return homepage;
}

// validate if `homepage` is a URL or path like and use just pathname
const validHomepagePathname = new URL(homepage, stubDomain).pathname;
return isEnvDevelopment
? homepage.startsWith('.')
? '/'
: validHomepagePathname
: // Some apps do not use client-side routing with pushState.
// For these, "homepage" can be set to "." to enable relative asset paths.
homepage.startsWith('.')
? homepage
: validHomepagePathname;
return isEnvDevelopment ? validHomepagePathname : homepage;
}

return '/';
Expand Down

0 comments on commit 66d3239

Please sign in to comment.