Skip to content
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

Fix $layout.svelte styles being imported in routes having $layout.reset.svelte #1356

Closed
wants to merge 2 commits into from

Conversation

DhyeyMoliya
Copy link

@DhyeyMoliya DhyeyMoliya commented May 6, 2021

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

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

This PR fixes #1100 , More Details :

  • The imported CSS inside $layout.svelte was applying in pages with $layout.reset.svelte as well.
  • This was happening because of manifest.js file having an export fallback = [c[0](), c[1]()].
  • Because of c[0]() the $layout.svelte was being executed at the start without it being required in a page or not.
  • This PR fixes this by changing the code to fallback = () => [c[0](), c[1]()]. So, the code does not execute at the start.

@Rich-Harris
Copy link
Member

Thank you. The current behaviour is deliberate though — the idea is that if a navigation fails, you need to have the root layout/error components available in memory (rather than a network request away, since a common reason for said failure is a lack of connectivity) so that things don't fall apart. This does cause a wasted request in the case where you initially hit a page with a layout reset, but on balance I think it's preferable.

@DhyeyMoliya
Copy link
Author

DhyeyMoliya commented May 6, 2021

Also, to reference some future issues similar to this.

The current code works like this :

When we load one page with main layout, the main layout css is appended to the head.

Then we go to another page with layout reset.svelte, then layout.reset's css is appended to head.

When we come back to the page with main layout.reset, the css of layout.reset is not removed by vite.

This PR is not able to solve that issue. Since it will require more deeper changes inside vite or hmr stuff.

If you say, I can discard this PR.

I have an alternative solution that completely works without any errors. Using <link> inside <svelte:head>. So the css is only added when that component is active. Without relating to layout and it's reset logic.

@Rich-Harris
Copy link
Member

Correct, links aren't removed. As long as you're not using ambiguous selectors (:global in components, or selectors without appropriate namespacing elsewhere) it shouldn't have an observable effect.

I have an alternative solution that completely works without any errors. Using <link> inside <svelte:head>. So the css is only added when that component is active. Without relating to layout and it's reset logic.

This will cause FOUC — there'll be a gap between the component rendering and the <link> loading — and is therefore unrecommended.

@Rich-Harris Rich-Harris closed this May 6, 2021
@DhyeyMoliya
Copy link
Author

This will cause FOUC — there'll be a gap between the component rendering and the <link> loading — and is therefore unrecommended.

Then I'm left with no solution to this problem.

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