[feat] render routes ending with `.html.svelte` to html files #1939

honai · 2021-07-17T22:12:26Z

Before submitting the PR, please make sure you do the following

It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
This message body should clearly illustrate what problems it solves.
Ideally, include a test that fails without this PR but passes with it.

Tests

Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

#1443 by changing the prerendering behavior. If a route ends with .html , write it as a file name instead of creating a folder.

Additional Info

I have to add flush option to glob to exactly check the dir structure.

changeset-bot · 2021-07-17T22:12:28Z

⚠️ No Changeset found

Latest commit: 169b0eb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

benmccann · 2021-07-18T20:07:15Z

Is there some documentation that should be added with this? Possibly in https://kit.svelte.dev/docs#ssr-and-javascript-prerender-route-conflicts ?

honai · 2021-07-19T08:43:53Z

I think we need some documentation about creating *.html.svelte .
The behavior of creating **/index.html.svelte is ambiguous and should be avoided (regardless of this PR).
This PR will cause users to create files like about.html.svelte. So it may be better to warn them that they should not create index.html.svelte ?

As for route conflicts caused by this PR are very edge case, as follows:

routes/about.html.svelte writes to build/about.html (after this PR)
routes/about.html/index.sveltewrites to build/about.html/index.html

As users would not include extensions in routing directory, I don't think we need to warn users about this.

benmccann · 2021-07-26T23:53:08Z

packages/kit/src/core/adapt/prerender.js

@@ -97,7 +97,7 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a
 	 */
 	function normalize(path) {
 		if (config.kit.trailingSlash === 'always') {
-			return path.endsWith('/') ? path : `${path}/`;
+			return path.endsWith('/') || path.endsWith('.html') ? path : `${path}/`;


Why just .html? What if the user wants to generate a sitemap.xml from sitemap.xml.svelte?

always no longer feels like a good name for this since it's not always doing it

This sort of feels like it should accept a function to let the user decide like #2008 and #2007. Then you could have it do whatever you want

Svelte files are designed to compose components made with HTML/CSS/JS so users won't generate files other than .html file, I think.

But endpoints like sitemap.xml.js are considered, so I think this option should be changed, as you say.

Can I change this option to something like never | ignore | (path: string) => boolean , in this PR? Or do you think it would be better to change it as a separate PR?

Yes, you're right that you'd use an endpoint

This probably shouldn't be a function after all. It gets stringified during build and could be confusing to users

I think a better logic here might be to do it only if the last path segment does not contain ., but let me ask the other maintainers what they think

Hmm, that might not be a great solution either. E.g. that wouldn't work with the URL https://bundlephobia.com/package/svelte@3.42.1

I think we just leave this as is and say you should not use the option always if you're generating .html files

benmccann · 2021-07-26T23:55:51Z

packages/kit/src/core/adapt/prerender.js

@@ -143,7 +143,7 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a
 			const is_html = response_type === REDIRECT || type === 'text/html';

 			const parts = path.split('/');
-			if (is_html && parts[parts.length - 1] !== 'index.html') {
+			if (is_html && !parts[parts.length - 1].endsWith('.html')) {


why did this need to change? if you encounter about.html why are you pushing index.html?

Before this change, paths like **/about.html are modified to **/about.html/index.html so I changed not to do so.

packages/kit/src/core/adapt/test/index.js

benmccann · 2021-07-26T23:58:44Z

index.html.svelte is ambiguous and should be avoided

What's ambiguous about it? I would think that it should create a file named index.html

benmccann · 2021-08-11T20:09:25Z

@honai will you be able to take a look at the questions / comments on this PR?

honai · 2021-08-12T08:07:37Z

I'm sorry for the late response. I replied to your comments.

What's ambiguous about it? I would think that it should create a file named index.html

"ambiguous" was an inappropriate description.
There are 3 possible cases in which routes are pre-rendered to build/about/index.html;

routes/about.svelte
routes/about/index.svelte
routes/about/index.html.svelte (new in this PR)

The documentation already says that routes are collated alphabetically . I don't think any additional explanation is necessary. What do you think?

benmccann · 2021-08-12T21:05:05Z

I talked to Rich about this feature briefly. His suggestion was that we create an option for generating about.html as opposed to about/index.html and that we not try to accomplish it by changing the file extension. I think that's nicer in a lot of ways because if you move web hosts you can just change the option in your config file rather than renaming all your files

benmccann · 2021-08-12T22:41:00Z

I'm going to go ahead and close this PR given the different approach that Rich suggested, but please feel free to send a PR with the updated approach and I'll take a look

honai added 2 commits July 18, 2021 05:45

Render routes ends with .html to non-indexed html

2701b5b

add test case for .html prerender

25481ee

honai mentioned this pull request Jul 17, 2021

Ability to prerender to about.html instead of about/index.html like Sapper #1443

Closed

benmccann force-pushed the master branch from 3e3e6ec to d0f2afd Compare July 25, 2021 14:15

benmccann reviewed Jul 26, 2021

View reviewed changes

packages/kit/src/core/adapt/test/index.js Outdated Show resolved Hide resolved

benmccann changed the title ~~Render routes ends with .html to non-indexed html~~ [feat] render routes ending with .svelte.html to html files Jul 27, 2021

This comment has been minimized.

Sign in to view

benmccann changed the title ~~[feat] render routes ending with .svelte.html to html files~~ [feat] render routes ending with .html.svelte to html files Jul 28, 2021

benmccann force-pushed the master branch from 06d2da2 to e8b391b Compare August 2, 2021 19:20

benmccann added the awaiting submitter label Aug 3, 2021

benmccann added the p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. label Aug 11, 2021

Rename test case to about.html

169b0eb

benmccann removed the awaiting submitter label Aug 12, 2021

benmccann closed this Aug 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feat] render routes ending with `.html.svelte` to html files #1939

[feat] render routes ending with `.html.svelte` to html files #1939

honai commented Jul 17, 2021 •

edited by benmccann

Loading

changeset-bot bot commented Jul 17, 2021 •

edited

Loading

benmccann commented Jul 18, 2021

honai commented Jul 19, 2021

benmccann Jul 26, 2021 •

edited

Loading

honai Aug 12, 2021

benmccann Aug 12, 2021

benmccann Aug 12, 2021

benmccann Jul 26, 2021

honai Aug 12, 2021

benmccann commented Jul 26, 2021

This comment has been minimized.

benmccann commented Aug 11, 2021

honai commented Aug 12, 2021

benmccann commented Aug 12, 2021

benmccann commented Aug 12, 2021

[feat] render routes ending with .html.svelte to html files #1939

[feat] render routes ending with .html.svelte to html files #1939

Conversation

honai commented Jul 17, 2021 • edited by benmccann Loading

Before submitting the PR, please make sure you do the following

Tests

Changesets

Additional Info

changeset-bot bot commented Jul 17, 2021 • edited Loading

⚠️ No Changeset found

benmccann commented Jul 18, 2021

honai commented Jul 19, 2021

benmccann Jul 26, 2021 • edited Loading

Choose a reason for hiding this comment

honai Aug 12, 2021

Choose a reason for hiding this comment

benmccann Aug 12, 2021

Choose a reason for hiding this comment

benmccann Aug 12, 2021

Choose a reason for hiding this comment

benmccann Jul 26, 2021

Choose a reason for hiding this comment

honai Aug 12, 2021

Choose a reason for hiding this comment

benmccann commented Jul 26, 2021

This comment has been minimized.

benmccann commented Aug 11, 2021

honai commented Aug 12, 2021

benmccann commented Aug 12, 2021

benmccann commented Aug 12, 2021

[feat] render routes ending with `.html.svelte` to html files #1939

[feat] render routes ending with `.html.svelte` to html files #1939

honai commented Jul 17, 2021 •

edited by benmccann

Loading

changeset-bot bot commented Jul 17, 2021 •

edited

Loading

benmccann Jul 26, 2021 •

edited

Loading