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

set Vite's publicDir and base options #5601

Closed
wants to merge 35 commits into from
Closed

set Vite's publicDir and base options #5601

wants to merge 35 commits into from

Conversation

benmccann
Copy link
Member

@benmccann benmccann commented Jul 18, 2022

Benefits:

  1. A lot less code in SvelteKit
  2. publicDir and base are now defined for Vite plugins (should help @userquin with his work supporting SvelteKit in vite-plugin-pwa)
  3. The Vite CLI now correctly includes the base URL, so you can Ctrl+click it in the terminal to be taken to the correct location

I would suggest closing #5115 with a suggestion that the issue be filed in the Vite repo

@changeset-bot
Copy link

changeset-bot bot commented Jul 18, 2022

🦋 Changeset detected

Latest commit: e1d80c8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@sveltejs/kit Patch
@sveltejs/adapter-node Patch

Not sure what this means? Click here to learn what changesets are.

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

@userquin
Copy link
Contributor

userquin commented Jul 18, 2022

@benmccann it is the webmanifest file, not the sw: pwa plugin will emit manifest.webmanifest asset in generateBundle but kit will copy it to _app folder.

We wrote the sw and workbox-*** assets to the client folder via workbox build or rollup.

@Rich-Harris
Copy link
Member

With this approach, the static assets get copied into .svelte-kit/output/client. Presumably we'd need to remove writeStatic from the adapter API if we did this

@userquin
Copy link
Contributor

userquin commented Jul 18, 2022

With this approach, the static assets get copied into .svelte-kit/output/client. Presumably we'd need to remove writeStatic from the adapter API if we did this

Why not just replace static with public folder? I don't need the assets to be written in the output folder.

I only need the publicDir to list the assets matching a globPattern.

The pwa plugin will use it to include the webmanifest icons in the sw's precache manifest.

@Rich-Harris
Copy link
Member

That's just how Vite works. Everything in public (or whatever else you set publicDir to — static in the typical SvelteKit app) gets copied into Vite's build output directory, which in our case is .svelte-kit/output/client

Comment on lines -476 to -479
test('Filenames are case-sensitive', async ({ request }) => {
const response = await request.get('/static.JSON');
expect(response.status()).toBe(404);
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was this test deleted?

Copy link
Member Author

@benmccann benmccann Jul 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vite does not check case sensitivity

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but many servers do, so it's essential that we do too — if we can't get Vite to be strict about casing it's a showstopper for this PR

@benmccann
Copy link
Member Author

Closing this for now as it doesn't seem feasible until maybe Vite 4 given current limitations in Vite

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

Successfully merging this pull request may close these issues.

4 participants