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

Invalid CSS: Space inserted after function interpolation #1368

Closed
PierreGUI opened this issue Oct 5, 2023 · 5 comments
Closed

Invalid CSS: Space inserted after function interpolation #1368

PierreGUI opened this issue Oct 5, 2023 · 5 comments
Labels
bug report 🦗 Issue is probably a bug, but it needs to be checked bundler: webpack 📦 Issue is related to webpack bundler needs: complete repro 🖥️ Issue need to have complete repro provided

Comments

@PierreGUI
Copy link

Environment

  • Linaria version: 5.0.2/5.0.3
  • Bundler (+ version): Webpack 5

Description

  1. Use a function interpolation
  2. Write CSS unit after the function
  3. Use variable interpolation on the next line
    ❌ There's a space between value and unit (invalid CSS)

Example:

export const FS = 12;
const getWidth = () => 10;

const H1 = styled.h1`
  width: ${getWidth}px;
  font-size: ${FS}px;
`;

Produces ->
image

Reproducible Demo

https://stackblitz.com/edit/linaria-bug-o9hmgr?file=src%2FTitle.tsx

@PierreGUI PierreGUI added bug report 🦗 Issue is probably a bug, but it needs to be checked needs: complete repro 🖥️ Issue need to have complete repro provided needs: triage 🏷 Issue needs to be checked and prioritized labels Oct 5, 2023
@github-actions github-actions bot added bundler: webpack 📦 Issue is related to webpack bundler and removed needs: triage 🏷 Issue needs to be checked and prioritized labels Oct 5, 2023
@layershifter
Copy link
Contributor

@PierreGUI the repro is partially broken:

image

However, usage of the proper version does not fix the bug :) It's present.


The transform() returns in result following:

result.rules {
  '.hhnsy8v': {
    cssText: '\n  width: var(--hhnsy8v-0);\n  font-size: 12\n                  px;\n',
    className: 'hhnsy8v',
    displayName: 'H1',
    start: { line: 10, column: 11, index: 220 }
  }
}

I traced it to the following:

// If it's a plain object or an array, convert it to a CSS string
cssText += stripLines(loc, toCSS(value));

  • toCSS() returns 12 (string)
  • stripLines() adds a newline that is considered as " "

This happens because:

// If the start and end line numbers aren't same, add new lines to span the text across multiple lines
if (loc.start.line !== loc.end.line) {
result += '\n'.repeat(loc.end.line - loc.start.line);
// Add extra spaces to offset the column
result += ' '.repeat(loc.end.column);
}

As stripLisnes() gets { start: { line: 11, column: 10 }, end: { line: 12, column: 18 } } as loc input.

@agriffis
Copy link
Contributor

@layershifter I have not dug into this personally, just curious if you continued down this debugging journey to figure out why stripLines receives those different start/end lines. It seems like they should be the same, given the input?

@agriffis
Copy link
Contributor

agriffis commented Oct 14, 2023

Oh, here it is:

lastTemplateElementLocation = item.loc;

lastTemplateElementLocation is only set for template elements. It isn't updated for expressions. As a result, it's still reflecting px;\n font-size: which started on the previous line.

@agriffis
Copy link
Contributor

This might actually be the same as #1141

@agriffis
Copy link
Contributor

I just added a PR for this, see #1374

Anber pushed a commit to agriffis/linaria that referenced this issue Nov 19, 2023
Anber added a commit that referenced this issue Nov 19, 2023
…1374)

* fix: space inserted after function interpolation (fixes #1141 #1368)

This fixes the failing test that was added in #1151.

* chore: changeset for #1374

---------

Co-authored-by: Anton Evzhakov <anton@evz.name>
@Anber Anber closed this as completed Nov 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report 🦗 Issue is probably a bug, but it needs to be checked bundler: webpack 📦 Issue is related to webpack bundler needs: complete repro 🖥️ Issue need to have complete repro provided
Projects
None yet
Development

No branches or pull requests

4 participants