Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Legacy build points to component local CSS at wrong path. #775

Closed
@antony

Description

@antony

The problem is that on route change (not if you directly visit a route), sapper attempts to load the route CSS from the /client/ directory, and looking at the chunk hashes, those files only exist in the legacy directory.

What this results in is your new route having only the CSS from the previously loaded routes.

This is very possibly related to #543 and #522 however I don't think it is the same.

I've made a simple reproduction here:

https://github.com/antony/sapper-ie/tree/feature/css-issue.

The issue can be triggered by running npm run ie to trigger a build + run, and then hitting the main index route, then clicking on the navigaiton to go to "/about".

In /index h1 is blue, in /about h1 is overriden to be red. The actual result is that in /about the h1 is black.

Activity

changed the title [-]Legacy build points to component local CSS u[/-] [+]Legacy build points to component local CSS at wrong path.[/+] on Jun 29, 2019
Conduitry

Conduitry commented on Jul 1, 2019

@Conduitry
Member

Having separate 'regular' and 'legacy' CSS builds doesn't really make sense - and is also not desirable if we have to do some sort of client-side test to see which CSS to load, because of the FOUC that would result in.

Server-rendered HTML is loading the correct, non-legacy .css files already. This is working fine.

Client-side navigation is using the load_css() function to load the CSS, and this is getting legacy chunk names, but is attempting to load them from client/blah.css rather than client/legacy/blah.css, which is a 404.

I'm confused about what exactly is happening with the __SAPPER_CSS_PLACEHOLDER that appears in the manifest files, but it would seem that this is getting replaced with chunk ids. The simplest solution might be to adjust this so that it includes the legacy/ in the paths when process.env.SAPPER_LEGACY_BUILD is set, but this is still duplication of CSS files (with different names) in client and client/legacy.

return JSON.stringify(process.env.SAPPER_LEGACY_BUILD ? result.chunks[route] && result.chunks[route].map(_ => `legacy/${_}`) : result.chunks[route]);

This seems to keep everything happy - but it's ugly for several reasons.

cristovao-trevisan

cristovao-trevisan commented on Jul 2, 2019

@cristovao-trevisan
Contributor

Hi,

this is also a big issue because it breaks SEO. If you are using the default error.svelte, which changes the page title to the status code, it will result into something like this:

image

My current (WORKING) workaround to this is to disable emitCss when in legacy mode, like so:

// ...
export default {
	client: {
		input: config.client.input(),
		output: config.client.output(),
		plugins: [
			replace({
				'process.browser': true,
				'process.env.NODE_ENV': JSON.stringify(mode)
			}),
			svelte({
				dev,
				preprocess: autoPreProcess(preprocessOptions),
				hydratable: true,
				emitCss: !legacy // <------------------- LOOK HERE
			}),
  // ...
antony

antony commented on Jul 2, 2019

@antony
MemberAuthor

@cristovao-trevisan this confuses me a bit. Web crawlers probably don't try too hard to parse the client (and parse the server-side, which works fine), but this bug also doesn't cause the client to show a 500 error. Additionally, if a smart crawler was to parse the client (google, for example) it uses a reasonably recent version of the chromium parser, so it would never receive the legacy build...

I'm confused at what you're seeing here - I feel like it probably doesn't relate to this issue? Unless you can explain a bit more?

cristovao-trevisan

cristovao-trevisan commented on Jul 2, 2019

@cristovao-trevisan
Contributor

@antony I would expect the same, but the result above is from google (search for "cristovao trevisan webpage", though the main result is correct because of the fix above, the other pages haven't been crawled until today).
Using the Google Search Console, the live test gets the correct result, but the google index still shows the <title>500</title>
Using the http://browsershots.org/ I could find which browser versions were giving the same result. Then I've download one of them (firefox 44.0) and confirmed that the issue was caused by the wrong css path for the legacy build

I've also find articles that say googlebot uses the latest version of chrome, so I get your point.

Maybe (more like 99.999% sure) the try catch block that decides to use the legacy build works that way in the googlebot environment.

Also, I'm hosting my page on github pages

antony

antony commented on Aug 27, 2019

@antony
MemberAuthor

Going to close this, since in my testing I've found that #775 seems to fix this issue succesfully!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @antony@cristovao-trevisan@Conduitry

        Issue actions

          Legacy build points to component local CSS at wrong path. · Issue #775 · sveltejs/sapper