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.

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

Jump to bottom

fix: missing tags inject fallback #5339

Merged
merged 2 commits into from
Oct 27, 2021
Merged

fix: missing tags inject fallback #5339

merged 2 commits into from
Oct 27, 2021

Conversation

patak-dev
Copy link
Member

Description

When there isn't a <head> or <body>, make sure to prepend after <html> or <!doctype html> if present.

This PR also makes all the regex case insensitive so we also match <HEAD> and <BODY>.

Before

<meta name="keywords" content="es modules">
<script type="module" crossorigin src="/assets/modulepreload-polyfill.0d94a2e8.js"></script>
<script type="module" crossorigin src="/assets/common.fde954c5.js"></script>
<script type="module" crossorigin src="/assets/main.48436d11.js"></script>
<link rel="stylesheet" href="/assets/common.e7d02c8e.css">
<link rel="stylesheet" href="/assets/main.ebd1b3ad.css">

<!DOCTYPE html>
<meta name="description" content="a vite app">

<html lang="en">
<body>
  <noscript><!-- this is prepended to body --></noscript>
...

After

<!DOCTYPE html>
<html lang="en">
<meta name="keywords" content="es modules">

<meta name="description" content="a vite app">

<script type="module" crossorigin src="/assets/modulepreload-polyfill.0d94a2e8.js"></script>
<script type="module" crossorigin src="/assets/common.fde954c5.js"></script>
<script type="module" crossorigin src="/assets/main.48436d11.js"></script>
<link rel="stylesheet" href="/assets/common.e7d02c8e.css">
<link rel="stylesheet" href="/assets/main.ebd1b3ad.css">

<body>
  <noscript><!-- this is prepended to body --></noscript>
...

Note: I tried to inject a <head> tag if it isn't present in #5327, but I think we should leave this to the user to avoid edge cases. This PR makes sure that the resulting html is well-formed without the tags.

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@antfu
Copy link
Member

antfu commented Oct 18, 2021

Do we need some tests to ensure it works stably in the future?

@patak-dev
Copy link
Member Author

Added some tests, and improve (again) the fallback. If there isn't a head tag but there is a body, append now will properly append before the body tag (instead of doing a prepend like before). And if there isn't a body tag but there is a head tag, prepend will properly place the tag after the head instead of (prepending in the html like before).

@patak-dev patak-dev requested a review from antfu October 18, 2021 19:17
@patak-dev patak-dev added the p3-minor-bug An edge case that only affects very specific usage (priority) label Oct 19, 2021
@patak-dev patak-dev added this to the 2.7 milestone Oct 22, 2021
@patak-dev patak-dev merged commit 3c44ac8 into main Oct 27, 2021
@patak-dev patak-dev deleted the fix/inject-fallback branch October 27, 2021 11:56
@ZYinMD ZYinMD mentioned this pull request Apr 26, 2022
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haoqunjiang haoqunjiang haoqunjiang approved these changes

@antfu antfu Awaiting requested review from antfu

Assignees
No one assigned
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Milestone
2.7
Development

Successfully merging this pull request may close these issues.
3 participants
@patak-dev @antfu @haoqunjiang