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

feat: add headTagInsertCheck warning #16555

Merged
merged 2 commits into from
May 29, 2024

Conversation

PengBoUESTC
Copy link
Contributor

Description

add warning message for applyHtmlTransforms.

there will be an abnormal scene, when we try to use some tags in head element, such as div.

here is an example:

<!DOCTYPE html>
<html>
  <head>
    <div>test div in head</div>
    <meta http-equiv="Content-Security-Policy" content="upgrade-insecure-requests">
  </head>

  <body>
    body
  </body>
</html>

in browser:
截屏2024-04-29 下午5 35 48

截屏2024-04-29 下午5 38 03

it will be better for developer, if vite provide some warn message.

Copy link

stackblitz bot commented Apr 29, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@PengBoUESTC PengBoUESTC requested a review from sapphi-red May 1, 2024 02:13
Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

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

I guess this warning won't happen that much, but I think the complexity is small enough.

packages/vite/src/node/plugins/html.ts Outdated Show resolved Hide resolved
packages/vite/src/node/plugins/html.ts Outdated Show resolved Hide resolved
if (!tags.length) return
const { logger } = ctx.server?.config || {}
const message = tags.reduce<string[]>((res, tagDescriptor) => {
if (!HeadElement[tagDescriptor.tag]) res.push(`<${tagDescriptor.tag}>`)
Copy link
Member

Choose a reason for hiding this comment

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

Note for other reviewers: I confirmed that custom elements are not allowed in <head>.

@sapphi-red sapphi-red added feat: html p2-nice-to-have Not breaking anything but nice to have (priority) labels May 1, 2024
@patak-dev
Copy link
Member

/ecosystem-ci run

@vite-ecosystem-ci
Copy link

@bluwy bluwy added this to the 5.3 milestone May 13, 2024
@bluwy bluwy merged commit 9f02a9f into vitejs:main May 29, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: html p2-nice-to-have Not breaking anything but nice to have (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants