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

fix(vite): don't prepend publicPath to public assets#2262

Closed
danielroe wants to merge 7 commits intomainfrom
fix/vite-base
Closed

fix(vite): don't prepend publicPath to public assets#2262
danielroe wants to merge 7 commits intomainfrom
fix/vite-base

Conversation

@danielroe
Copy link
Copy Markdown
Member

@danielroe danielroe commented Dec 2, 2021 •
edited
Loading

🔗 Linked issue

resolves nuxt/nuxt#12892

❓ Type of change

  • 📖 Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

This PR sets base to be / or cdnURL, if set (previously _nuxt), and set assetsDir instead to _nuxt. This means we have the desired rewriting in the HTML for our assets: static assets aren't rewritten, but generated assets are. The only hack is that we then have to rewrite the client manifest to remove this assetsDir prefix (because the bundle renderer will handle re-inserting it for us).

Ideally we would have support in vite directly for this.

It produces a file structure like:

.output/public/_nuxt/entry-74f24f8b.mjs
.output/public/_nuxt/entry.bb100772.css
.output/public/_nuxt/logo-2.148d9522.svg
.output/public/_nuxt/manifest.json
.output/public/_nuxt/vendor-1887291f.mjs
.output/public/robots.txt
.output/public/logo.svg

and in the HTML:

<!DOCTYPE html>
<html data-head-attrs="">

<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width, initial-scale=1">
  <meta name="head:count" content="2">
  <link rel="modulepreload" href="/_nuxt/entry-74f24f8b.mjs" as="script">
  <link rel="preload" href="/_nuxt/entry.bb100772.css" as="style">
  <link rel="modulepreload" href="/_nuxt/vendor-1887291f.mjs" as="script">
  <link rel="prefetch" href="/_nuxt/logo-2.148d9522.svg">
  <link rel="stylesheet" href="/_nuxt/entry.bb100772.css">
</head>

<body data-head-attrs="">
  <div id="__nuxt">
    <div class="app" data-v-12ae6901><img src="/logo.svg" height="40" data-v-12ae6901><img
        src="/_nuxt/logo-2.148d9522.svg" height="40" data-v-12ae6901>
      <h1 class="greeting" data-v-12ae6901>Hello, <br data-v-12ae6901>Nuxt 3! </h1>
    </div>
  </div>
  <script>window.__NUXT__ = { data: {}, state: { hello: "Hello" }, _errors: {}, serverRendered: true, config: { app: { basePath: "\u002F", assetsPath: "\u002F_nuxt\u002F", cdnURL: null } } }</script>
  <script type="module" src="/_nuxt/entry-74f24f8b.mjs" defer></script>
</body>

</html>

📝 Checklist

  • I have linked an issue or discussion.
  • update bridge vite configuration too
  • I have updated the documentation accordingly.

@danielroe danielroe added bug Something isn't working vite labels Dec 2, 2021
@danielroe danielroe requested review from antfu and pi0 December 2, 2021 12:24
@danielroe danielroe self-assigned this Dec 2, 2021
@netlify
Copy link
Copy Markdown

netlify bot commented Dec 2, 2021 •
edited
Loading

✔️ Deploy Preview for nuxt3-docs canceled.

🔨 Explore the source changes: 4f2188c

🔍 Inspect the deploy log: https://app.netlify.com/sites/nuxt3-docs/deploys/61b733306c25060007168dcf

@danielroe danielroe changed the title fix: don't prepend publicPath to public assets fix(vite): don't prepend publicPath to public assets Dec 2, 2021
@danielroe danielroe marked this pull request as draft December 2, 2021 12:29
pi0
pi0 reviewed Dec 13, 2021
View reviewed changes
Comment thread packages/vite/src/vite.ts Outdated
}
},
base: nuxt.options.build.publicPath,
base: nuxt.options.dev ? nuxt.options.build.publicPath : nuxt.options.app.cdnURL || '/',
Copy link
Copy Markdown
Member

@pi0 pi0 Dec 13, 2021

Choose a reason for hiding this comment

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

Can we please not depend on CDN URL in vite config? CDN URL should not be considered in the app build step. It is mostly a deployment configuration. (what if we always default prod base to / ?)

@danielroe danielroe marked this pull request as ready for review December 13, 2021 11:49
@daniele-sarnari
Copy link
Copy Markdown

daniele-sarnari commented Dec 14, 2021 •
edited by danielroe
Loading

nuxt/nuxt#12892

@daniele-sarnari daniele-sarnari mentioned this pull request Dec 14, 2021
Closed
@danielroe
Copy link
Copy Markdown
Member Author

danielroe commented Dec 14, 2021

@DrewLiberty Are you getting this behaviour with this PR?

@daniele-sarnari
Copy link
Copy Markdown

daniele-sarnari commented Dec 14, 2021

@DrewLiberty Are you getting this behaviour with this PR?

Haven't tried, how can I do it? it is not merged into main so a npm update won't work.

@entity
Copy link
Copy Markdown
Contributor

entity commented Dec 21, 2021

@DrewLiberty Are you getting this behaviour with this PR?

How can we try it via npm?

@danielroe danielroe mentioned this pull request Jan 10, 2022
Merged
7 tasks
@pi0
Copy link
Copy Markdown
Member

pi0 commented Jan 11, 2022

Closing in the favor of #2655

@pi0 pi0 closed this Jan 11, 2022
@pi0 pi0 deleted the fix/vite-base branch January 11, 2022 18:00
@danielroe danielroe added the 3.x label Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@antfu antfu Awaiting requested review from antfu

1 more reviewer

@pi0 pi0 pi0 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@danielroe danielroe

Labels

3.x bug Something isn't working vite

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Generated image urls for files in the public directory result in 404's when running the built server.

4 participants

@danielroe @daniele-sarnari @entity @pi0