Skip to content

base configuration option #194

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

Closed
wants to merge 17 commits into from
Closed

base configuration option #194

wants to merge 17 commits into from

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Nov 17, 2023

The base option (for #42), can be set to /path/to/root/; defaults to /.

supported in observable dev: serves the project under the base, and redirects / to the new base to help the user. However it 404s on any other URL, to avoid false positives.

Fil added 6 commits November 17, 2023 11:37
supported in `observable dev`: serves the project under the base, and redirects / to the new base to help the user. However it 404s on any other URL, to avoid false positives.

will be used in `observable build` to help the 404 page and more to come.
@Fil Fil requested review from mbostock and mythmon November 17, 2023 13:25
@Fil Fil changed the title **base** config option base configuration option Nov 17, 2023
Base automatically changed from fil/relative-file to main November 17, 2023 17:13
@Fil
Copy link
Contributor Author

Fil commented Nov 18, 2023

I initially thought that this would be needed to build a useful 404.html page (#174). But here's the twist: if the site is served from https://example.com/base/ it is most probable that the server will not use the generated 404.html file as a template for its 404 errors (since it doesn't live at the server root).

In other words: we don't need the base configuration option for #174. Of course, if we merge #205 (done) we should honor the base option on the 404 page both in preview and build, for consistency, but keep in mind that the only case where that page will be useful in production is when the base is /.

@mythmon mythmon removed their request for review December 1, 2023 22:46
@mythmon
Copy link
Member

mythmon commented Dec 1, 2023

It looks like Platform won't need this afterall, since everything uses relative URLs. Given that, I'm going to step down as reviewer.

@Fil Fil marked this pull request as draft December 1, 2023 22:58
@Fil
Copy link
Contributor Author

Fil commented Dec 4, 2023

Do we need this at all? Maybe not…

@Fil Fil closed this Dec 4, 2023
@Fil Fil deleted the fil/base branch February 2, 2024 09:55
@Fil
Copy link
Contributor Author

Fil commented Feb 9, 2024

@mythmon do we fix this on the platform or should I revive this issue? (We need to fix https://observablehq.com/framework/404 (#721) in one way or the other.)

@mythmon
Copy link
Member

mythmon commented Feb 9, 2024

I don't think we need this on the platform. Plus we are moving towards not having a separate build for deploy, so we wouldn't be able to to inject it anyways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants