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

Static assets that are used as a background in css emit a warning #913

Open
husseincak opened this issue Mar 25, 2022 · 19 comments
Open

Static assets that are used as a background in css emit a warning #913

husseincak opened this issue Mar 25, 2022 · 19 comments

Comments

@husseincak
Copy link

Bug report

Expected behavior
Correctly styled rendering on the server side without any warnings when you add a static url from the server assets to the css props (like a background image)

Observed behavior
If you import an image and you add it as a background-image via css or style props, the console shows a warning,

Screenshots
image

Steps involved to reproduce the problem

  1. Create a frontity repository: npx frontity create
  2. Add any image the theme package
  3. Import it on the component index
  4. Add it as a background of any html element

I have created a repository with a hyper-simplified version of frontity where there is only a div with a static image as background: https://github.com/husseincak/testing/

Info about the problem
If you downgrade the package @frontity/core to the v 1.14.3, the warning no longer appears:

  • @frontity/core: ^1.15.1 => 1.14.3
    If i console.log the asset i get two different urls on the client and on the server:
  • Background URL on client: /static/images/image.jpg
  • Background URL on the server: __webpack_public_path__images/image.jpg

Maybe the problem is because the two urls are diferent so the component is being rendered as a different component on the server and on the client

@luisherranz
Copy link
Member

Interesting.

  • Background URL on client: /static/images/image.jpg
  • Background URL on the server: __webpack_public_path__images/image.jpg

It is related to this PR, where we started populating the dynamic public path with __webpack_public_path__ in the server: #909

The problem is that setting the correct public path dynamically at request time seemed not possible. I guess we need to find a way to do it. If anyone has experience doing this or wants to give it a try, please do so 🙂

@luisherranz
Copy link
Member

We can try two things:

  • Remove __webpack_public_path__ declaration from here and add it here (with the correct public path value).
  • Try modifying the public path value here, but using __webpack_require__.p instead of __webpack_public_path__ (explained in more detail here.

@orballo
Copy link
Collaborator

orballo commented May 15, 2022

Dropping this here because the previous solutions do not work.
https://github.com/agoldis/webpack-require-from
Not sure if this plugin would work on server side though. It looks like it only tries to find the variables on window.

EDIT: This comment says it works: https://stackoverflow.com/a/58222901. I'll try this next week.

@orballo
Copy link
Collaborator

orballo commented May 15, 2022

@luisherranz I tried the plugin mentioned in my previous comment but once it is added to the package.json and I run npm i, I get a bunch of TS errors:

Screenshot 2022-05-15 at 20 14 15

No idea why this is happening.

I don't know if the option publicPath: 'auto' of Webpack 5 would solve our issue. I think we discussed it but I don't remember what was the conclusion. Maybe instead of going for a custom fix, migrating Webpack 4 to 5 is an option?

@luisherranz
Copy link
Member

What about the things I mentioned in #913 (comment)? Didn't they work?

@orballo
Copy link
Collaborator

orballo commented May 17, 2022

@luisherranz Nop they didn't. It looks like webpack needs the variable set at the entry point, before everything else, so changing the variable inside a function has no effect :/

@luisherranz
Copy link
Member

I see. I'll try to test webpack-require-from and see if I can find a way to solve the TypeScript errors.

@luisherranz
Copy link
Member

@orballo you're going to look into this, right?

@orballo
Copy link
Collaborator

orballo commented May 19, 2022

@luisherranz that is correct :)

@orballo
Copy link
Collaborator

orballo commented May 20, 2022

@luisherranz Apparently the plugin works well with chunks, but not with other assets. There is an issue open on that: agoldis/webpack-require-from#8

Is there a way to manipulate the emotion compiled string before sending it to the client?

@luisherranz
Copy link
Member

I'm not sure how to proceed, to be honest. I've been taking a look myself, but I couldn't find any solution to changing those values at runtime. The variables are there, but changing them doesn't seem to affect the output.

@orballo
Copy link
Collaborator

orballo commented May 24, 2022

@luisherranz I think we can get rid of this issue preprocessing the CSS.

If we change the way we SSR the styles, like explained here, and we use @emotion/cache, which apparently has an option to hook Stylis middleware, we might be able to replace the value of __webpack_public_path__ in the CSS before the class is generated, and get rid of the warning.

@luisherranz
Copy link
Member

That sounds promising 🙂

@orballo
Copy link
Collaborator

orballo commented May 26, 2022

The last solution I proposed doesn't work either. Classes are generated with the styles string before being preprocessed in the cache, so even if it is replaced in the cache, it still triggers the warning.

EDIT: I made it work with a huuuuge hack that I need to test in more scenarios to see if it works.
EDIT 2: I was replacing the hashes of the classes, but it doesn't work on more complex components. So, at this point, other than trying Webpack 5 and publicPath: "auto" and hope for a miracle, I think there is no way of fixing this, at least not at the Emotion level.

@luisherranz
Copy link
Member

Frontity v2 and switch to Vite? 😄

I don't know, to be honest. My only idea is to debug the final server.js file to try to find the exact moment where Webpack is passing the public path to Emotion, and see if there's something we can do at that point. I'm not sure if it's worth the effort, though.

Does publicPath: auto solve the problem?

@orballo
Copy link
Collaborator

orballo commented May 30, 2022

@luisherranz I don't know, but we are updating to Webpack 5 after the WordCamp because we are running already into some conflicts with other packages. So, I'll try after that.

Frontity v2 sounds good, but I have a lot on my plate right now 😆

@ghost
Copy link

ghost commented Sep 23, 2022

Hi guys!
I have faced with this issue too...
Have you found a solution for this?

@orballo
Copy link
Collaborator

orballo commented Sep 23, 2022

Hi @SergiusNahnoinyi! I'm afraid I couldn't figure out a fix. I was going to try with the publicPath: 'auto' setting in Webpack 5, but it didn't work out of the box and I didn't have more resources to dedicate to this issue.

You could give it a try, but I think it is a very long shot. Just in case you want to try, there is a beta branch of Frontity with webpack updated to v5. #925 (comment)

@meaghanbass
Copy link

Is there any update on this?

@orballo orballo removed their assignment Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants