homepage in package.json breaks react-create-app if it contains a fragment identifier (#foo)

[michaelmcmillan]

This is a subtle bug which causes create-react-app to strip a character from the homepage URL and then crash. It was discovered by @cyrille-arundo, @geevb and myself.

Steps to reproduce

1. npx create-react-app my-app
2. Add a URL with a fragment identifier to package.json's homepage: https://domain.com/page#hello"
3. Run npm start
4. Watch it blow up in all its glory.

Compiled successfully! You can now view my-app in the browser. Local: http://localhost:3000/pag On Your Network: http://10.10.10.86:3000/pag Note that the development build is not optimized. To create a production build, use yarn build. URIError: Failed to decode param '/%PUBLIC_URL%/favicon.ico' at decodeURIComponent (<anonymous>) at decode_param (/Users/michaelmcmillan/reproduce/my-app/node_modules/express/lib/router/layer.js:172:12) [...] 

Pay attention to the two URLs http://localhost:3000/pag and http://10.10.10.86:3000/pag. They're both missing the last character (e).

Why is this happening?
There's a lot of technical acrobatics going on when create-react-app tries to determine its URL.

I've added a failing test case to demonstrate how packages/react-dev-utils/getPublicUrlOrPath returns a path without a trailing slash (/) if the URL contains a fragment identifier. This becomes a problem later because because packages/react-scripts/scripts/start.js assumes the URL always contain a trailing slash. Instead it ends up stripping the last character from the URL.

Feedback
Also, debugging this was unnecessarily painful. Lines like the following, with three-way ternaries, make me reconsider my career options.

 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; 

This is taken from packages/react-dev-utils/getPublicUrlOrPath.

[facebook-github-bot]

Hi @michaelmcmillan!

Thank you for your pull request and welcome to our community.We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

[stale]

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

[joaofranciscosantos]

#8686
This patch might fix your problem too.

[michaelmcmillan]

@joaofranciscosantos Nice! Maybe you should take the test I've written in this PR to make sure it works on #8686?

[stale]

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

[stale]

This pull request has been automatically closed because it has not had any recent activity. If you have a question or comment, please open a new issue. Thank you for your contribution!