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

Jump to bottom

fix: injected tags indentation #5000

Merged
merged 2 commits into from Sep 23, 2021
Merged

fix: injected tags indentation #5000

merged 2 commits into from Sep 23, 2021

Conversation

patak-dev
Copy link
Member

@patak-dev patak-dev commented Sep 20, 2021 •
edited

Description

We merged #4227 to improve indentation of injected tags, but that PR assumes that the head is indented with two spaces. The approach in this PR is similar to #1068, but also takes into account the case when the head isn't indented.

The only caveat is that if the head isn't indented, it assumes two spaces for indentation. If it is indented, then it will properly respect tabs. We could improve this further by detecting the indentation with another regex, but I think that would be already too complex and fragile. We may need to provide a config option for this, but I prefer to avoid proposing something without a request from users. I think this approach is enough for the moment.

Current indentation and spacing when <head> and <body> start at the beginning of the line (real example from playground/html):

<!DOCTYPE html>
<html lang="en">
<head>
  <meta name="description" content="a vite app">
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>Test HTML transforms</title>
  <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">
    <meta name="keywords" content="es modules">
  </head>
<body>

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

  <p class="inject">This is injected</p>
    <p class="build">This is injected only during build</p>
    <noscript><!-- this is appended to body --></noscript>
</body>
</html>

After this PR:

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

  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>Test HTML transforms</title>
  <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">
  <meta name="keywords" content="es modules">
</head>
<body>
  <noscript><!-- this is prepended to body --></noscript>

  ...

  <p class="inject">This is injected</p>
  <p class="build">This is injected only during build</p>
  <noscript><!-- this is appended to body --></noscript>
</body>
</html>

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@Shinigami92 Shinigami92 added the p3-minor-bug An edge case that only affects very specific usage (priority) label Sep 21, 2021
@antfu antfu merged commit 4b84c0d into main Sep 23, 2021
aleclarson pushed a commit to aleclarson/vite that referenced this pull request Nov 8, 2021
@patak-dev @aleclarson
fix: injected tags indentation (vitejs#5000)
5471bc4
@antfu antfu deleted the fix/injected-tags-indentation branch December 24, 2021 05:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antfu antfu antfu left review comments

@Shinigami92 Shinigami92 Shinigami92 approved these changes

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@patak-dev @Shinigami92 @antfu