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

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

Open
nolanlawson opened this issue Apr 5, 2024 · 15 comments
Open
Labels
bug Up for grabs Issues that are relatively small, self-contained, and ready for implementation

Comments

@nolanlawson
Copy link
Contributor

nolanlawson commented Apr 5, 2024

<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: 8136599

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 nolanlawson added bug Up for grabs Issues that are relatively small, self-contained, and ready for implementation labels Apr 5, 2024
@HermanBide
Copy link

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

@nolanlawson
Copy link
Contributor Author

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

@HermanBide
Copy link

what folder/file can i find this code ?

@nolanlawson
Copy link
Contributor Author

I believe the buggy code is here:

const important = value.endsWith('!important');

In order to reproduce the test failure, you will also need to check out 8136599 and run DISABLE_STATIC_CONTENT_OPTIMIZATION=1 yarn test:karma.

@HermanBide
Copy link

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

@nolanlawson
Copy link
Contributor Author

nolanlawson commented Apr 16, 2024

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

git clone git@github.com: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.

@HermanBide
Copy link

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

@nolanlawson
Copy link
Contributor Author

@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)

@HermanBide
Copy link

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

@nolanlawson
Copy link
Contributor Author

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

@HermanBide
Copy link

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

@nolanlawson
Copy link
Contributor Author

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.

@SaurabhMulay999
Copy link
Contributor

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.

@nolanlawson
Copy link
Contributor Author

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. 🙂

@SaurabhMulay999
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Up for grabs Issues that are relatively small, self-contained, and ready for implementation
Projects
None yet
Development

No branches or pull requests

3 participants