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

Invalid CSS file name caused by brackets and + #5843

Closed
eahrold opened this issue Aug 6, 2022 · 10 comments · Fixed by vitejs/vite#9737 or #6398
Closed

Invalid CSS file name caused by brackets and + #5843

eahrold opened this issue Aug 6, 2022 · 10 comments · Fixed by vitejs/vite#9737 or #6398
Labels
bug Something isn't working vite
Milestone

Comments

@eahrold
Copy link

eahrold commented Aug 6, 2022

Describe the bug

When a wild card route is used, and the application is build with the adapter-node the resulting .css file has square brackets in the file name which should be escaped.

This is not a problem with serving with node directly, however when proxied through some non-js routers the route is considered invalid, as per various specs. (i.e. I'm serving the node application in a docker container, and have a proxy server intercepting requests and forwarding them to the node server)

and if you pass the route through encodeURI it too escapes the brackets

encodeURI("http://localhost:3000/_app/immutable/assets/[...allRoutes]-b0a89eda.css")

// http://localhost:3000/_app/immutable/assets/%5B...allRoutes%5D-b0a89eda.css

Demo of the URL format when used in Java

Reproduction

https://github.com/eahrold/svelte-kit-invalid-css-path

The problem .css file is here
https://github.com/eahrold/svelte-kit-invalid-css-path/tree/main/build/client/_app/immutable/assets

The corresponding .js file works and looks to be processed differently.
https://github.com/eahrold/svelte-kit-invalid-css-path/tree/main/build/client/_app/immutable/pages

The browser request doesn't encode the url

invalid-route-used-directly

Logs

No response

System Info

System:
    OS: macOS 12.2
    CPU: (8) arm64 Apple M1
    Memory: 97.22 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.15.1 - ~/.nvm/versions/node/v16.15.1/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v16.15.1/bin/yarn
    npm: 8.11.0 - ~/.nvm/versions/node/v16.15.1/bin/npm
  Browsers:
    Chrome: 104.0.5112.79
    Firefox: 102.0.1
    Safari: 15.3
  npmPackages:
    @sveltejs/adapter-node: next => 1.0.0-next.85 
    @sveltejs/kit: next => 1.0.0-next.403 
    svelte: ^3.44.0 => 3.49.0 
    vite: ^3.0.0 => 3.0.4

Severity

serious, but I can work around it

Additional Information

No response

@benmccann benmccann added the vite label Aug 8, 2022
@benmccann
Copy link
Member

This was previously happening with JS files, but was fixed for that case: #1974 (comment)

Anyway, this likely isn't generated by SvelteKit, but by Vite. Please file the issue there if you'd like it to be seen by anyone likely to fix it: https://github.com/sveltejs/kit#bug-reporting

@eahrold
Copy link
Author

eahrold commented Aug 12, 2022

@benmccann did you mean to put the sveltekt bug-reporting link above?
Also I couldn't quite suss out the PR for the JS fix, did that happen on the vite side? Can you provide any more pointers to where I should look?

@benmccann
Copy link
Member

benmccann commented Aug 12, 2022

@eahrold that is the link I mean to share. Did you read what's there? It describes how to report issues to Vite that affect SvelteKit projects. I'm not exactly sure which PR changed the behavior in Vite for JS files

It could also be from Rollup. Understanding whether it's Vite or Rollup responsible for this logic would be a good first step

@denovodavid
Copy link

I'm also running into this now. With the new routing, all route related files are prepended with + which would need encoding to be valid in a URL. Vite transforms +page.js to _page.js on build, but doesn't seem to do the same with CSS.

My project works fine in dev and preview, but when I deploy to Cloudfront and S3 I get error 404 on all route related CSS files, such as +layout.css, as they still have the + in them. Manually replacing the + with %2b in the URL serves the correct files.

@eahrold Did you find any related issue on the Vite repo? Or know of any workaround?

@benmccann benmccann changed the title Invalid css file name (for url path) generated in catch-all route. Invalid CSS file name caused by brackets and + Aug 18, 2022
@benmccann benmccann added the bug Something isn't working label Aug 18, 2022
@benmccann benmccann added this to the 1.0 milestone Aug 18, 2022
@Rich-Harris
Copy link
Member

Rich-Harris commented Aug 18, 2022

We've narrowed it down to this function in Vite, which is supplying an invalid filename to Rollup (which has no choice but to use the supplied fileName verbatim). I think we just need to replace all special characters here with underscores to mirror how Rollup handles renaming for JS modules:

const name = basename.slice(0, -extname.length)

@mattosborn
Copy link

mattosborn commented Aug 19, 2022

I made a quick workaround for my case using vite's plugin system to override the config sveltekit sets. Add to vite.config plugins after sveltekit. This only fixes + prefixed css assets but could be adapted easily for other cases.

/** @type {import('vite').UserConfig} */
const config = {

  plugins: [
    sveltekit(),

    // <workaround for https://github.com/sveltejs/kit/issues/5843>
    {
      config(config) {
        const original = config.build.rollupOptions.output.assetFileNames;
        config.build.rollupOptions.output.assetFileNames = assetInfo => {
          const match = assetInfo.name.match(/\/\+(.*)\.css$/);
          return match ? original.replace('[name]', match[1]) : original;
        };
        return config;
      }
    },
    // </workaround>

  ]
};

@JanTrichter
Copy link

Can this issue be closed or is the bug still relevant?

@Conduitry
Copy link
Member

The fix in Vite has not been released yet. We should keep this open until we confirm that the fix works.

@souravjamwal77
Copy link

I made a quick workaround for my case using vite's plugin system to override the config sveltekit sets. Add to vite.config plugins after sveltekit. This only fixes + prefixed css assets but could be adapted easily for other cases.

/** @type {import('vite').UserConfig} */
const config = {

  plugins: [
    sveltekit(),

    // <workaround for https://github.com/sveltejs/kit/issues/5843>
    {
      config(config) {
        const original = config.build.rollupOptions.output.assetFileNames;
        config.build.rollupOptions.output.assetFileNames = assetInfo => {
          const match = assetInfo.name.match(/\/\+(.*)\.css$/);
          return match ? original.replace('[name]', match[1]) : original;
        };
        return config;
      }
    },
    // </workaround>

  ]
};

Hi @mattosborn, I'm getting below error when trying to run the application locally

error when starting dev server:
TypeError: Cannot read properties of undefined (reading 'assetFileNames')
    at Object.config (file:///home/souravjamwal77/Desktop/projects/truviss-svelte/vite.config.js.timestamp-1661601113233.mjs:8:61)
    at resolveConfig (file:///home/souravjamwal77/Desktop/projects/truviss-svelte/node_modules/vite/dist/node/chunks/dep-0fc8e132.js:62629:33)
    at async createServer (file:///home/souravjamwal77/Desktop/projects/truviss-svelte/node_modules/vite/dist/node/chunks/dep-0fc8e132.js:59183:20)
    at async CAC.<anonymous> (file:///home/souravjamwal77/Desktop/projects/truviss-svelte/node_modules/vite/dist/node/cli.js:699:24)

It's coming from Vite config.

@Rich-Harris Rich-Harris modified the milestones: 1.0, whenever Aug 27, 2022
@stephane-vanraes
Copy link
Contributor

@souravjamwal77 the issue with the invalid file names only happens during build and not during development, so the workaround should only be applied during build:

{
  config(config) {
    const original = config.build.rollupOptions.output.assetFileNames;
    config.build.rollupOptions.output.assetFileNames = (assetInfo) => {
    const match = assetInfo.name.match(/\/\+(.*)\.css$/);
    return match ? original.replace('[name]', match[1]) : original;
  };
  return config;
  },
  apply: 'build'
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working vite
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants