withastro / adapters

Home for Astro's core maintained adapters
69 stars 41 forks source link

cloudflare adapter breaks rollupOptions output filenames #334

Closed mschoeffmann closed 3 months ago

mschoeffmann commented 3 months ago

Astro Info

Astro                    v4.12.2
Node                     v20.14.0
System                   macOS (arm64)
Package Manager          pnpm
Output                   hybrid
Adapter                  @astrojs/cloudflare
Integrations             @astrojs/mdx
                         @astrojs/sitemap

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

Using rollupOptions like entryFileNames, chunkFileNames and assetFileNames for reasons (withastro/astro#7469, withastro/astro#8549, withastro/astro#5787, ...) moves the files from ./dist folder to ./dist/_render.js which results in 404 after deploying.

      rollupOptions: {
        output: {
          entryFileNames: '_static/entry.[hash].js',
          chunkFileNames: '_static/chunk.[hash].js',
          assetFileNames: '_static/asset.[hash][extname]',
        },
      },

check out the min. reproduction stackblitz and run the command npm run build first with the above config, and second with the above lines commented out. there it only affects the .css file but on larger projects it affects all bundled chunks and assets.

not working: css is _worker.js subfolder:

Screenshot 2024-07-20 at 02 35 00

working without output filename config: css is in _astro subfolder:

Screenshot 2024-07-20 at 02 35 45

What's the expected result?

bundled files in ./_static as configured. works without adapter and 'static' output mode.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-tzkvup?file=src%2Flayouts%2FLayout.astro%2Castro.config.mjs&on=stackblitz

Participation

Changes: edited screenshot annotations and fixed issue links

alexanderniebuhr commented 3 months ago

@mschoeffmann Thank you for the report. Is this 'dist/_static/chunk.[hash].js' you expected output path?

If so I'm afraid that it won't be supported, since all SSR related files must be under the path 'dist/_worker.js'/

mschoeffmann commented 3 months ago

@alexanderniebuhr the expected path would be './dist/_static/chunk.[hash].js'. In the example above, the CSS file is not available for clients because it is in the _worker.js/ subdirectory. Even without a subdirectory, just changing the filename to chunk.[hash].js like this moves the files to the _worker subdirectory.

alexanderniebuhr commented 3 months ago

Okay understood. I don't think it would be working with chunks. Only static assets like css, etc.

Also I don't think this is Cloudflare specific, but rather a generic issue, when the option build.client & build.server is used.

I also think their is a different configuration for that, let me see if I can find it.

mschoeffmann commented 3 months ago

I think the adapter already knows which files belong to the client and which belong to the client. Because without using the output options, all the files are in the right place.

Okay understood. I don't think it would be working with chunks. Only static assets like css, etc.

But right now, also assets are not working. The css file for example lands in _worker.js subdirectory.

alexanderniebuhr commented 3 months ago

I'll try to see if the same behavior happens with the Node adapter, to see if this is a core issue.

mschoeffmann commented 3 months ago

Update: I just tried with the latest @astrojs/vercel, @astrojs/netlify and @astrojs/node ... and the results were equal.

I also tried this with a ready-to-migrate web project and noticed especially .css files and .jpg files were inside the wrong folder.

mschoeffmann commented 3 months ago

I'll try to see if the same behavior happens with the Node adapter, to see if this is a core issue.

Hahaha - coincidence - just done... https://github.com/withastro/adapters/issues/334#issuecomment-2243470114

matthewp commented 3 months ago

It's pretty unlikely that this is going to work. Netlify and Vercel need files to be in special folders for their deployment. It's not really configurable.

alexanderniebuhr commented 3 months ago

The node adapter has the same issue when using build.server & build.client: https://stackblitz.com/edit/github-tzkvup-oqmfkz?file=astro.config.mjs

alexanderniebuhr commented 3 months ago

@mschoeffmann Any reason why you can't use https://docs.astro.build/en/reference/configuration-reference/#buildassets to change the name of _astro, for the static assets?

matthewp commented 3 months ago

Overriding vite in your config applies to the server-side and client-side code. I think you only want client-side. So using the build.assets config option like @alexanderniebuhr suggested, or using the astro:build:setup hook to set options only for client-side builds is the way to go: https://docs.astro.build/en/reference/integrations-reference/#astrobuildsetup

mschoeffmann commented 3 months ago

@matthewp Yes, true. But I'm not trying to change the whole platform-specific paths.

My goal is to:

  1. remove confusing filenames like 404.[hash].css or about-us.[hash].css for globals styles (see links below or initial issue)
  2. put all static assets inside https://example.com/_static/ instead of https://example.com/_astro/

Links about the filenames:

Thanks for the quick response @matthewp and @alexanderniebuhr. I'll try the mentioned workarounds and follow up in a few minutes ...

matthewp commented 3 months ago

It's not really a workaround, those are the methods of adjusting output filenames for client-side code. vite is global configuration. Let us know if it works out!

alexanderniebuhr commented 3 months ago

Yeah I guess for "2." build.assets: "_static" is the correct way :)

mschoeffmann commented 3 months ago

@matthewp yep - sorry :-)

I wrote a quick integration to test this:

export const genericClientFilenames = () => {
  let integration = {
    name: 'generic-client-filenames',
    hooks: {
      'astro:build:setup': ({ vite, target }) => {
        if (target === 'client') {
          console.log(vite.build.rollupOptions.output)
          vite.build.rollupOptions.output.entryFileNames =
            '_static-CLIENT/entry.[hash].js'
          vite.build.rollupOptions.output.chunkFileNames =
            '_static-CLIENT/chunk.[hash].js'
          vite.build.rollupOptions.output.assetFileNames =
            '_static-CLIENT/asset.[hash][extname]'
          console.log(vite.build.rollupOptions.output)
        }
        if (target === 'server') {
          console.log(vite.build.rollupOptions.output)
          //// COMMENTED OUT BECAUSE ENTRY AND CHUNK FOR SERVER BUILD ARE GENERATED ON THE FLY BY FUNCTIONS --->
          // vite.build.rollupOptions.output.entryFileNames =
          //   '_static/entry.[hash].js'
          // vite.build.rollupOptions.output.chunkFileNames =
          //   '_static/chunk.[hash].js'
          //// COMMENTED OUT BECAUSE ENTRY AND CHUNK FOR SERVER BUILD ARE GENERATED ON THE FLY BY FUNCTIONS ---<
          vite.build.rollupOptions.output.assetFileNames =
            '_static-SERVER/asset.[hash][extname]'
          console.log(vite.build.rollupOptions.output)
        }
      },
    },
  }
  return integration
}

export default genericClientFilenames

The result is:

So for me it looks like the asset files were generated only for server target?

the -client and -server folder suffix as well as the console statements are for better debugging.

mschoeffmann commented 3 months ago

Quick reproduction: https://stackblitz.com/edit/github-tzkvup-wqls1g?file=src%2Fpages%2Findex.astro&on=stackblitz

Screenshot 2024-07-22 at 21 24 08

mschoeffmann commented 3 months ago

I found a working solution now by overwriting the each *FileNames key of the output object with a function to replace [name] with a string like entry, chunk or asset. Also I just change the filename so the build.assets option mentioned by @alexanderniebuhr still works.

I'm not sure if this is the best way, and it still looks strange that somehow css assets are generated in "server" target. But well ¯_(ツ)_/¯

Maybe I'll release an integration package on npm for this one ... Reading the mentioned threads sounds like more people want to get rid of the confusing filenames.

// ./src/integrations/generic-client-filenames.ts

import type { AstroIntegration } from 'astro'
import type { PreRenderedChunk, PreRenderedAsset, OutputOptions } from 'rollup'

export const genericClientFilenames = () => {
  let integration: AstroIntegration = {
    name: 'generic-client-filenames',
    hooks: {
      'astro:build:setup': ({ vite }) => {
        const usedOutputOptions = vite.build.rollupOptions.output as OutputOptions
        const originalOutputOptions = Object.assign(
          {},
          vite.build.rollupOptions.output,
        ) as OutputOptions
        usedOutputOptions.entryFileNames = (chunkInfo: PreRenderedChunk) => {
          const originalNames =
            typeof originalOutputOptions.entryFileNames === 'function'
              ? originalOutputOptions.entryFileNames(chunkInfo)
              : originalOutputOptions.entryFileNames
          return originalNames.replace('[name]', 'entry')
        }
        usedOutputOptions.chunkFileNames = (chunkInfo: PreRenderedChunk) => {
          const originalNames =
            typeof originalOutputOptions.chunkFileNames === 'function'
              ? originalOutputOptions.chunkFileNames(chunkInfo)
              : originalOutputOptions.chunkFileNames
          return originalNames.replace('[name]', 'chunk')
        }
        usedOutputOptions.assetFileNames = (assetInfo: PreRenderedAsset) => {
          const originalNames =
            typeof originalOutputOptions.assetFileNames === 'function'
              ? originalOutputOptions.assetFileNames(assetInfo)
              : originalOutputOptions.assetFileNames
          return originalNames.replace('[name]', 'asset')
        }
      },
    },
  }
  return integration
}

export default genericClientFilenames
// ./astro.config.mjs

import genericClientFilenames from './src/integrations/generic-client-filenames'
...

export default defineConfig({
...
  integrations: [genericClientFilenames()],
  build: {
    assets: '_static',
  },
...
})
alexanderniebuhr commented 3 months ago

Great to hear you found a way to solve your use-case, I'll close the issue for now.

mschoeffmann commented 3 months ago

For reference: I created an integration npm package for this, available from https://www.npmjs.com/package/astro-generic-build-filenames

alexanderniebuhr commented 3 months ago

That's great. I suggest posting this to the showcase channel in Discord too :)