lwc icon indicating copy to clipboard operation
lwc copied to clipboard

Inline styles do not parse `! important` correctly due to extra whitespace

Open nolanlawson opened this issue 1 year ago • 15 comments

<div lwc:spread={fake} style="color: red ! important"></div>

The above should render with color: red having a priority of important. However, it loses the important priority because, when the static content optimization is not active, we do not parse the important priority correctly (due to the whitespace: ! important).

Repro Karma test: 81365992e

This test will succeed when the static content optimization is enabled, but it will fail when using DISABLE_STATIC_CONTENT_OPTIMIZATION=1. The reason it succeeds with the optimization is because we leave the style attribute alone and don't try to format it.

See also: #3548.

nolanlawson avatar Apr 05 '24 21:04 nolanlawson

has this issue been resolved ? is no can you assign it to me ?

HermanBide avatar Apr 09 '24 17:04 HermanBide

@HermanBide Feel free to grab it if you'd like! No need for assignments :)

nolanlawson avatar Apr 09 '24 18:04 nolanlawson

what folder/file can i find this code ?

HermanBide avatar Apr 10 '24 16:04 HermanBide

I believe the buggy code is here:

https://github.com/salesforce/lwc/blob/1dba7074b4963ade06e872ef6cbb74b955aca377/packages/%40lwc/template-compiler/src/codegen/helpers.ts#L188

In order to reproduce the test failure, you will also need to check out https://github.com/salesforce/lwc/commit/81365992e05585daadc4b17a9184ab7bfdb9b54a and run DISABLE_STATIC_CONTENT_OPTIMIZATION=1 yarn test:karma.

nolanlawson avatar Apr 15 '24 15:04 nolanlawson

I ran DISABLE_STATIC_CONTENT_OPTIMIZATION=1 yarn test:karma and did not see any failure. also can we connect on discord.

HermanBide avatar Apr 16 '24 16:04 HermanBide

Hi @HermanBide, you may need to check out the commit first. Try running these commands:

git clone [email protected]:salesforce/lwc.git
cd lwc
git checkout 81365992e05585daadc4b17a9184ab7bfdb9b54a
yarn
DISABLE_STATIC_CONTENT_OPTIMIZATION=1 yarn test:karma

Unfortunately we do not have a Discord/Slack community server at this time.

nolanlawson avatar Apr 16 '24 21:04 nolanlawson

Chrome Headless 107.0.5304.87 (Linux x86_64): Executed 3154 of 3459 (skipped 305) SUCCESS (15.41 secs / 13.799 secs)
TOTAL: 3154 SUCCESS
 >  NX   Successfully ran target test for project @lwc/integration-karma (26s)
 
Done in 26.04s.

I get this when i run

DISABLE_STATIC_CONTENT_OPTIMIZATION=1 yarn test:karma

HermanBide avatar Apr 17 '24 16:04 HermanBide

@HermanBide Are you running on Windows? On Windows you may need to test in Windows Subsystem for Linux, or else find another way to set the global DISABLE_STATIC_CONTENT_OPTIMIZATION env var (e.g. npx cross-env DISABLE_STATIC_CONTENT_OPTIMIZATION=1 yarn test:karma).

On my machine (MacOS), I am able to run the above commands verbatim and I get the error:

Chrome Headless 123.0.6312.124 (Mac OS 10.15.7) rendering important-styles should render !important styles correctly FAILED
        Expected '' to be 'red'.
            at <Jasmine>
            at UserContext.<anonymous> (rendering/important-styles/test/rendering/important-styles/index.spec.js:11:53 <- rendering/important-styles/index.spec.js:63:53)
        Expected '' to be 'important'.
            at <Jasmine>
            at UserContext.<anonymous> (rendering/important-styles/test/rendering/important-styles/index.spec.js:12:56 <- rendering/important-styles/index.spec.js:64:56)
        Expected '' to be 'red'.
            at <Jasmine>
            at UserContext.<anonymous> (rendering/important-styles/test/rendering/important-styles/index.spec.js:11:53 <- rendering/important-styles/index.spec.js:63:53)
        Expected '' to be 'important'.
            at <Jasmine>
            at UserContext.<anonymous> (rendering/important-styles/test/rendering/important-styles/index.spec.js:12:56 <- rendering/important-styles/index.spec.js:64:56)
        Expected '' to be 'red'.
            at <Jasmine>
            at UserContext.<anonymous> (rendering/important-styles/test/rendering/important-styles/index.spec.js:11:53 <- rendering/important-styles/index.spec.js:63:53)
        Expected '' to be 'important'.
            at <Jasmine>
            at UserContext.<anonymous> (rendering/important-styles/test/rendering/important-styles/index.spec.js:12:56 <- rendering/important-styles/index.spec.js:64:56)

nolanlawson avatar Apr 17 '24 17:04 nolanlawson

Yes i am running on window. i do not get those errors

HermanBide avatar Apr 22 '24 14:04 HermanBide

Please try running npx cross-env DISABLE_STATIC_CONTENT_OPTIMIZATION=1 yarn test:karma. It should work on Windows to set the environment variable.

nolanlawson avatar Apr 22 '24 16:04 nolanlawson

ca we zoom ? i honestly do not see the error when i run that command in the terminal

HermanBide avatar Apr 25 '24 18:04 HermanBide

I'm sorry, but we do not have the resources to help external contributors one-on-one. I would recommend using Windows Subsystem for Linux, or a Mac/Linux device. I don't believe any of the regular contributors to LWC use a Windows machine, and so it's possible things will be broken. 😕

I just checked, and these instructions are still reproducing the error for me, on a Mac.

nolanlawson avatar Apr 25 '24 19:04 nolanlawson

Hey Nolan, I also have tried to test this, It's still reproducing the error for me on Mac. tho for the plain html, It's working.

<div style="color:red ! important" ></div>

But for above example which you have provided:

<div lwc:spread={fake} style="color: red ! important">{statement}</div>

It's not even applying the color attribute to the text.

SourabhMulay avatar Apr 26 '24 14:04 SourabhMulay

Correct, the lwc:spread={fake} causes LWC to change from its "static content optimization" mode to the regular mode. In that mode, ! important is not handled correctly. That's the bug. 🙂

nolanlawson avatar Apr 26 '24 15:04 nolanlawson

Thanks @nolanlawson for the explanation. I'll hit a try to resolve.

SourabhMulay avatar Apr 26 '24 16:04 SourabhMulay

Fixed by https://github.com/salesforce/lwc/pull/4329

nolanlawson avatar Jul 01 '24 19:07 nolanlawson