Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Consider setting output.publicPath to '/' by default #1171

Closed
edmorley opened this issue Oct 12, 2018 · 3 comments
Closed

Consider setting output.publicPath to '/' by default #1171

edmorley opened this issue Oct 12, 2018 · 3 comments
Assignees
Milestone

Comments

@edmorley
Copy link
Member

edmorley commented Oct 12, 2018

The output.publicPath option controls the URL prefix used to refer to generated assets. webpack defaults it to '' (@neutrinojs/web currently overrides this to './' - however that's equivalent), which means that the generated HTML refers to assets using relative paths, eg index.js.

However if sites are using the HTML 5 pushState history API, and have page routes under nested paths (eg /foo/bar or even just /foo/), then even if they correctly configure their webserver/hosting redirects (ie to serve index.html for requests that would otherwise 404), requests to assets will fail.

For example, without publicPath manually set in their .neutrinorc.js, the page at https://health.graphics/android/ (which is really the contents of /index.html) would make a request to the non-existent:
https://health.graphics/android/runtime.1ca11f96.js

...rather than the actual URL of:
https://health.graphics/runtime.1ca11f96.js

This affects both production and development (the latter possible due to a webpack-dev-server bug).

The downside to setting publicPath by default, is that for apps that don't use pushState and are not hosted at the site root (eg foo.com/docs/ or user.github.io/repo/), then they'll have to override publicPath otherwise their site won't work at all in production. However if these sites were using pushState, then they already had to be setting publicPath to the correct path (eg '/docs/') otherwise reloads wouldn't have worked anyway. (And at least the new failure mode is more obvious)

So the question is, for @neutrinojs/web do we:

  1. Do nothing (ie continue to set historyApiFallback: true giving the impression pushState will work, when it really won't, without customising publicPath)
  2. Just add some docs about needing to set publicPath for pushState sites to work with direct links
  3. Same as (2) but also stop setting historyApiFallback and add it to the list of things we say they have to set in the docs (at least the default config would be less misleading)
  4. Give a warning/info message if projects don't explicitly set a publicPath. We could also have create-project prompt them for it. Though this would be unnecessary noisy for all projects that are at the site root.
  5. Same as (4) but make it an error if not set. Though this is pretty disruptive.
  6. Change the default to '/' so that historyApiFallback: true works, and just add docs for people who are not hosting at site root. We could optionally prompt during create-project and if they add a custom value that gets set, otherwise no addition to .neutrinorc.js.
  7. Same as (6) but make it a top-level neutrino option alongside source, output etc, to improve visibility (we could also pick an easier to understand name for the setting)

For reference, CRA defaults to '/':
https://facebook.github.io/create-react-app/docs/deployment#building-for-relative-paths
...and has this console output:
https://github.com/facebook/create-react-app/blob/50607682ef1c5e28c158a613051ddafb30567b72/packages/react-dev-utils/printHostingInstructions.js#L47-L56

And vue-cli defaults to '/' as well:
https://cli.vuejs.org/config/#baseurl

I think (6) or (7) would be my preference.

@eliperelman / @timkelty / @helfi92 / @armenzg or anyone else - thoughts? :-)

@edmorley edmorley added this to the v9 milestone Oct 12, 2018
@eliperelman
Copy link
Member

So far I am leaning towards 6.

@timkelty
Copy link
Contributor

Same here – leaning towards 6.

@edmorley
Copy link
Member Author

edmorley commented Oct 12, 2018

One downside to using '/' is that it means loading the HTML file from the local filesystem will no longer work (#175 (comment)).

However sites that use pushState will break with SecurityError: The operation is insecure when opened from the local filesystem even now. And presumably the list of "only available over secure contexts (such as HTTPS and localhost but not file:///)" Web APIs is only going to increase over time - meaning loading from the local filesystem is not really a workflow that's viable any more.

Given that, and that users can always override, I don't think it's a blocker.

@edmorley edmorley self-assigned this Oct 14, 2018
edmorley added a commit that referenced this issue Oct 23, 2018
Since otherwise apps that use the HTML 5 pushState history API won't
work when deployed, even with the necessary server rewrite rules
configured.

Apps that are deployed from a non-domain-root location (such as those
hosted on GitHub pages) will now need to explicitly set `publicPath`
themselves, however this is the case with CRA and vue-cli too.

Fixes #1171.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

3 participants