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

Icomoon font will not be rendered with emotion enabled #40385

Closed
1 task done
dominikmatt opened this issue Sep 9, 2022 · 8 comments · Fixed by #61426
Closed
1 task done

Icomoon font will not be rendered with emotion enabled #40385

dominikmatt opened this issue Sep 9, 2022 · 8 comments · Fixed by #61426
Assignees
Labels
locked SWC Related to minification/transpilation in Next.js.

Comments

@dominikmatt
Copy link

dominikmatt commented Sep 9, 2022

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
  Platform: darwin
  Arch: x64
  Version: Darwin Kernel Version 21.6.0: Sat Jun 18 17:07:25 PDT 2022; root:xnu-8020.140.41~1/RELEASE_X86_64
Binaries:
  Node: 14.15.0
  npm: 7.11.1
  Yarn: 1.22.18
  pnpm: N/A
Relevant packages:
  next: 12.3.1-canary.0
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

Describe the Bug

Background
Since we switched to nextjs@12 we only got minified css classes on development for our @emotion styles.

The Problem
Now we tried to enable emotion with the compiler.emotion = true configuration. With that emotion works well except our icomoon font.
The icomoon font will be loaded and added to the styles... but not rendered. I'm not sure what the problem is exactly, for me all the things looks well.

Expected Behavior

Icons should be rendered as a font

Link to reproduction

https://github.com/dominikmatt/nextjs-emotion

To Reproduce

  • Add emotion to your nextjs project
  • Add a emotion component and define all the icons inside of it
  • Enable emotion on next-configuration
const nextConfig = {
  compiler: {
    emotion: true, // Change to false to get the icon rendered
  },
}
@dominikmatt dominikmatt added the bug Issue was opened via the bug report template. label Sep 9, 2022
@balazsorban44 balazsorban44 added SWC Related to minification/transpilation in Next.js. kind: bug and removed bug Issue was opened via the bug report template. labels Sep 9, 2022
@rubytree33
Copy link
Contributor

Taking a look at this.

@rubytree33
Copy link
Contributor

It breaks because if compiler.emotion = true, the inline stylesheet in index.js is emitted with content: "\\e904" rather than content: "\e904". This is what it looks like in the original source in index.js. And no other escaping works in this template string.

So my guess is this is introduced at compile time by next-swc/crates/emotion, specifically here which is called to modify the template string after styles.tagname. I think a layer of string escaping is introduced here implicitly in type conversion, so I'm trying an explicit conversion.

@rubytree33
Copy link
Contributor

Looking into it a little more, it seems that all those conversions in swc don't escape anything.

But after some investigation and looking at related issues (emotion-js/emotion#1755, emotion-js/emotion#2802) I found #38301 which is apparently the root of the problem.

Tagged template strings basically have two forms: the "cooked" one which translates escape sequences, and the "raw" one which leaves the string exactly as it appears in the source. (See here for more info.) So I think this is the line responsible:

let minified = minify_css_string(&q.raw, index == 0, index == args_len - 1);

q.raw here selects the raw form, and it seems the transformations which come later (in this function) never attempt to "cook" the string.

@kdy1 Are you still working on #38301? If not I'll try making a pr that should resolve both.

@kdy1
Copy link
Member

kdy1 commented Sep 10, 2022

No I'm not working on it

@dominikmatt
Copy link
Author

@rubytree33 When can we expect a fix here? Looks like it is already fixed but not merged…

@rubytree33
Copy link
Contributor

That's up to the maintainers, since I can't merge the PR myself.

@kdy1 kdy1 assigned kdy1 and unassigned kdy1 Dec 4, 2023
@kdy1 kdy1 self-assigned this Jan 30, 2024
@kdy1
Copy link
Member

kdy1 commented Jan 31, 2024

swc-project/plugins#260 should fix this issue

kdy1 added a commit that referenced this issue Jan 31, 2024
### What?

Update SWC crates

### Why?

To apply the latest bugfixes

### How?

Fixes #40385



Closes PACK-2332
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked SWC Related to minification/transpilation in Next.js.
Projects
None yet
4 participants