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

[bug]: Duplicate inline <style>s after 'composes' #3547

Closed
3 of 8 tasks
alecarg opened this issue Nov 10, 2021 · 8 comments
Closed
3 of 8 tasks

[bug]: Duplicate inline <style>s after 'composes' #3547

alecarg opened this issue Nov 10, 2021 · 8 comments
Assignees
Labels
bug Something isn't working Issue: Needs Update Additional information is require, waiting for response Progress: ready for grooming triage-done

Comments

@alecarg
Copy link

alecarg commented Nov 10, 2021

Describe the bug

It would seem like an exact duplicate of the expected <style> tag produced by a venia css file is being added to the DOM. This duplicate is only added after one composes a class from that venia css file; if the composes is removed, the duplicate <style> tag disappears. The problem is that the duplicate <style> tag is being added after our own custom <style> tag (resulting from our own custom .css file we've added to our project). Because of specificity, the duplicate <style> tag takes precedence over our own custom <style> tag based on DOM positioning, thus not allowing us to extend venia.

See this for the resulting DOM: https://i.ibb.co/3sTQSYJ/Screenshot-2021-11-10-at-09-44-07.png
See this for the resulting styles: https://i.ibb.co/4PCY6zb/Screenshot-2021-11-09-at-11-49-37.png

To reproduce

Steps to reproduce the behavior:

  1. Use the targetables mechanism to add a custom .css file import to a venia js component (in my case, cartTrigger.js) – similar to https://dev.to/chrisbrabender/simplifying-styling-in-pwa-studio-1ki1 . This means we will be getting this in the browser:
const classes = useStyle(defaultClasses, customClasses, props.classes);  // customClasses is imported from our own project
  1. In our custom .css file, add an existing venia class name ie. .trigger and compose venia's .trigger using composes.
  2. Observe the styles associated to the .trigger element: 3 set of styles are applied, 2 of which are identical and are applied to the element with varying levels of specificity – as well as the single set of custom styles we added, which does not take precedence over the duplicate styles.

Expected behavior

Only 2 set of styles are applied, with the venia ones being less specific than our custom ones.

Screenshots

See above.

Possible solutions

This was working fine in v9~, but broke after we upgraded to v12.0.

It would seem useStyle does the same shallowMerge as mergeClasses used to do, so I've discarded that.

One thing I found is that altering the order of defaultClasses and customClasses (the params fed to useStyle) yields a different result: the duplicate <style> tag is added, but the custom <style> tag isn't, meaning it makes matters worse. The weird part: removing the composes still removes the duplicate <style> tag, almost as if our custom .css file was being taken into consideration after all.

--

Also, I tried to change the name of our own .css file so that it does not contain .module.css, but that did not help either.

Debug Report

    Found 10 @magento dependencies in yarn.lock
    @magento/pwa-buildpack @ 11.0.0
    @magento/upward-js @ 5.2.0
    @adobe/apollo-link-mutation-queue @ 1.0.2
    @magento/babel-preset-peregrine @ 1.1.0
    @magento/eslint-config @ 1.5.2
    @magento/pagebuilder @ 7.0.1
    @magento/peregrine @ 12.1.0
    @magento/pwa-theme-venia @ 1.0.0
    @magento/upward-security-headers @ 1.0.5
    @magento/venia-ui @ 9.1.0

  ℹ  Inspecting Magento Backend
    Not using sample backend.
    Backend is UP!

  ℹ  Inspecting System
    OS: Darwin Kernel Version 18.6.0: Thu Apr 25 23:16:27 PDT 2019; root:xnu-4903.261.4~2/RELEASE_X86_64
    Node Version: v16.13.0
    NPM Version: 8.1.0

Please complete the following device information:

  • Reproducible on all devices
  • Magento version is 2.4.2, PWA is 12.0

Please let us know what packages this bug is in regards to:

  • venia-concept
  • venia-ui
  • pwa-buildpack
  • peregrine
  • pwa-devdocs
  • upward-js
  • upward-spec
  • create-pwa
@alecarg alecarg added the bug Something isn't working label Nov 10, 2021
@m2-assistant
Copy link

m2-assistant bot commented Nov 10, 2021

Hi @alecarg. Thank you for your report.
To speed up processing of this issue, make sure that you provided sufficient information.

Add a comment to assign the issue: @magento I am working on this


@supernova-at
Copy link
Contributor

@magento export issue to JIRA project PWA as Bug

@github-jira-sync-bot
Copy link

✅ Jira issue https://jira.corp.magento.com/browse/PWA-2412 is successfully created for this GitHub issue.

@supernova-at
Copy link
Contributor

Thanks for the report, @alecarg - we'll investigate!

@0m3r
Copy link
Contributor

0m3r commented Nov 22, 2021

I have the same problem and look like it appear after v12.1 upgrade

I think It's maybe because addImport use prependSource

 prependSource(insert) {
        return this.spliceSource({ at: 0, insert });
 }

prependSource adds custom CSS import at the start of a file

import ar7HIClasses from "@vendor/pwa-theme-name/lib/components/CartPage/ProductListing/quantity.module.css";
import React, { Fragment } from 'react';
...
import defaultClasses from './quantity.module.css';
...
const classes = useStyle(defaultClasses, ar7HIClasses, props.classes);

@yaroslav-qlicks
Copy link

Same issue still reproduced on latest PWA v12.4

@glo80771 glo80771 self-assigned this Jul 27, 2023
@glo80771
Copy link
Contributor

Hi @alecarg ,

Can you please provide the css file content to add it in the component which you used for reproducing so that it would be helpful for reproducing the issue?

@glo80771 glo80771 added the Issue: Needs Update Additional information is require, waiting for response label Jul 28, 2023
@glo82145
Copy link
Collaborator

glo82145 commented May 8, 2024

As We have no update on this issue Hence we are closing it

@glo82145 glo82145 closed this as completed May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Issue: Needs Update Additional information is require, waiting for response Progress: ready for grooming triage-done
Projects
None yet
Development

No branches or pull requests

8 participants