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

AspectRatio still does not apply the ratio value #1008

Closed
1 of 3 tasks
omarkhatibco opened this issue Jul 10, 2023 · 7 comments · Fixed by #1012 or #1096
Closed
1 of 3 tasks

AspectRatio still does not apply the ratio value #1008

omarkhatibco opened this issue Jul 10, 2023 · 7 comments · Fixed by #1012 or #1096

Comments

@omarkhatibco
Copy link

omarkhatibco commented Jul 10, 2023

Description

Hey again,

I just tried the new AspectRatio pattern after the last bug, right now only the padding-bottom value is not working.

it works if I don't provide a value and it use the default one 4 / 3 but when I provide something else such as 16 / 9 or even 1 / 2, I see the generated class but it does not apply for some reason

Link to Reproduction

here

Steps to reproduce

Check Aspect Ratio component

JS Framework

React TS

Panda CSS Version

0.6.0

Browser

Latest

Operating System

  • macOS
  • Windows
  • Linux

Additional Information

No response

@astahmer
Copy link
Collaborator

my bad my bad, thanks for catching this (again)

@omarkhatibco
Copy link
Author

Hey @astahmer,

I'm sorry to inform you that it's still not working 🙈
check the codesandbox here

@astahmer
Copy link
Collaborator

It was ! It even is still working in the tests.. I even tried pasting your exact sample

Screenshot 2023-07-23 at 23 34 57

We use ts-evaluator for those cases instead of a simple eval, as a BinaryExpression could contain TS specific syntax like 1 + (2 as number) etc

So I tried debugging the issue using your repo and I found that the typescript node.kind (223) on the BinaryExpression (16 / 7) wasn't matching the one from your typescript version 5.1.6 (which is also the one we use in the panda repo)
Screenshot 2023-07-23 at 23 33 17

Screenshot 2023-07-23 at 23 33 41

Then I found out we (in panda repo) have upgraded the TS version to 5.1.6 without upgrading ts-morph to 19.0.0 which support 5.1, so there is a mismatch since our ts-morph is at 18.0.8 which only supports 5.0.2

After that I tried updating ts-morph in my local panda repo and despite having the correct node.kind (225), ts-evaluator utility function isExpression is still returning false when encoutering BinaryExpression

My final guess is that ts-evaluator fails because of the exact same reasons listed above, it only supports until typescript: 4.9.4

I'm not sure why it works in the tests tho ? also, not sure how to resolve that (and the future issues that will arise if ts-evaluator isn't updated 😢 )

@omarkhatibco
Copy link
Author

omarkhatibco commented Jul 24, 2023

@astahmer,
No worries, I just wanted to mention the issue, for now, I could fix maybe using css vars

could you please reopen the issue so I can keep tracking it?

@astahmer astahmer reopened this Jul 24, 2023
@astahmer
Copy link
Collaborator

yes ofc, and thank you as usual for reporting those, it helps a lot! 🙏

@astahmer
Copy link
Collaborator

this should be fine once #1055 gets merged

@omarkhatibco
Copy link
Author

Hey @astahmer, I can confirm that it's working now, thanks for your help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants