Skip to content

fix: inline proxies for virtual html in transformIndexHtml #7993

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
May 4, 2022
Merged

fix: inline proxies for virtual html in transformIndexHtml #7993

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1d34d51
fix(importAnalysis): early null return on undefined
saurabhdaware May 2, 2022
ac05f25
chore: revert isDebug change
saurabhdaware May 2, 2022
2a764af
fix: url may be '/' run resolveId get real path
poyoho May 3, 2022
ccda81b
feat: add try catch to resolveId
saurabhdaware May 3, 2022
a2a0d4f
feat: handle html name in getHtmlFilename
saurabhdaware May 3, 2022
eb6bfc7
feat: update ssr-html example with SSR docs code
saurabhdaware May 3, 2022
e5eedeb
fix: error
poyoho May 3, 2022
cced917
fix: pass for hmr
poyoho May 3, 2022
b650fd6
chore: update, wip
patak-dev May 3, 2022
1bf08ff
fix: handle js and css proxies for virtual html
patak-dev May 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions packages/playground/ssr-html/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ const DYNAMIC_SCRIPTS = `
<script type="module" src="/src/app.js"></script>
`

const DYNAMIC_STYLES = `
<style>
h1 {
background-color: blue;
}
</style>
`

async function createServer(
root = process.cwd(),
isProd = process.env.NODE_ENV === 'production'
Expand Down Expand Up @@ -44,13 +52,18 @@ async function createServer(

app.use('*', async (req, res) => {
try {
let [url] = req.originalUrl.split('?')
if (url.endsWith('/')) url += 'index.html'
const url = req.originalUrl
Copy link
Contributor Author

@saurabhdaware saurabhdaware May 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Updated this code to be a bit similar to SSR docs example

This one is failing with unresolved path error now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#7993 (comment) this can solve the issue. Is that expected behaviour?

Was wondering what exactly is the solution-

  • Update docs to add the solution that @poyoho mentioned
  • Or fix the resolve logic and automatically handle index.html paths?

Copy link
Member

Choose a reason for hiding this comment

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

We can update the docs in v3 if we think that is the correct way forward, but this was working before and documented so we should merge a fix in 2.9 that works as it is currently documented:

const url = req.originalUrl

Copy link
Member

Choose a reason for hiding this comment

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

You can watch resolve plugin to know how vite internal resolveId work. So you can set the package.json>main field to avoid the error.

Copy link
Member

Choose a reason for hiding this comment

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

@poyoho do you see an issue with adding this logic:

 let [url] = req.originalUrl.split('?') 
 if (url.endsWith('/')) url += 'index.html'

to transformIndexHtml?

Or would it later fail in render (https://vitejs.dev/guide/ssr.html#setting-up-the-dev-server)

    // 4. render the app HTML. This assumes entry-server.js's exported `render`
    //    function calls appropriate framework SSR APIs,
    //    e.g. ReactDOMServer.renderToString()
    const appHtml = await render(url)

Since we are asking users to add the index.html by hand, why not add it ourselves when needed so we avoid breaking these cases? Maybe we could emit a warning later saying that in v3 this will not be supported and they have to pass a complete path?

if (url.startsWith('/favicon.ico')) {
return res.status(404).end('404')
}
const htmlLoc = url === '/' ? resolve('index.html') : `${url}.html`
let template = fs.readFileSync(htmlLoc, 'utf-8')

const htmlLoc = resolve(`.${url}`)
let html = fs.readFileSync(htmlLoc, 'utf8')
html = html.replace('</body>', `${DYNAMIC_SCRIPTS}</body>`)
html = await vite.transformIndexHtml(url, html)
template = template.replace(
'</body>',
`${DYNAMIC_SCRIPTS}${DYNAMIC_STYLES}</body>`
)
const html = await vite.transformIndexHtml(url, template)

res.status(200).set({ 'Content-Type': 'text/html' }).end(html)
} catch (e) {
Expand Down
20 changes: 18 additions & 2 deletions packages/vite/src/node/server/middlewares/indexHtml.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,12 @@ function getHtmlFilename(url: string, server: ViteDevServer) {
if (url.startsWith(FS_PREFIX)) {
return decodeURIComponent(fsPathFromId(url))
} else {
let htmlFileName = url.slice(1)
if (!htmlFileName) {
htmlFileName = 'index.html'
}
return decodeURIComponent(
normalizePath(path.join(server.config.root, url.slice(1)))
normalizePath(path.join(server.config.root, htmlFileName))
)
}
}
Expand Down Expand Up @@ -190,7 +194,19 @@ const devHtmlHook: IndexHtmlTransformHook = async (

await Promise.all(
styleUrl.map(async ({ start, end, code }, index) => {
const url = filename + `?html-proxy&${index}.css`
// NOTE: ssr url may be '/' run resolveId to get the real path
// let resolvedId: string | undefined
// try {
// resolvedId = (await server!.pluginContainer.resolveId(filename))?.id
// } catch (err) {
// // If fails to resolve filename, try resolving <filename>/index.html
// resolvedId = (
// await server!.pluginContainer.resolveId(
// path.join(filename, 'index.html')
// )
// )?.id
// }
const url = `${filename}?html-proxy&${index}.css`

// ensure module in graph after successful load
const mod = await moduleGraph.ensureEntryFromUrl(url, false)
Expand Down