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

Unable to generate style names when compiled under :simple optimizations #7

Closed
sansarip opened this issue Mar 25, 2020 · 5 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@sansarip
Copy link

sansarip commented Mar 25, 2020

Global styling via defglobal seems to not work with :simple optimization. The weird thing is that global styles work just fine in :advanced optimization.

Here's a lightweight, sample project for your viewing convenience:
https://github.com/sansarip/yacwat-simple

You can change the :optimization level in the prod.cljs.edn file to :simple and then compile a jar that you can run with lein prod - more detailed instructions in the README.

Is this intentional / am I doing something wrong?

@dhleong
Copy link
Owner

dhleong commented Mar 26, 2020

Thanks for the report! This is indeed odd, and it's definitely not intended that we don't work under :simple optimization. I guess I've only tested in dev and :advanced modes, so I've never seen this before. In fact, it looks like none of the styling works correctly under :simple optimization.

Looking at your sample project, it seems like the factory functions don't have a .name property when compiled under :simple optimizations. It's unclear why, though; the code generated in dev and :simple mode seems to be the same. But this is the cause—we use the name of the style function to generate the style ID to know which element to update in the DOM when/if the style changes....

@dhleong dhleong added the bug Something isn't working label Mar 26, 2020
@dhleong dhleong changed the title Optimization issues with defglobal Unable to generate style names when compiled under :simple optimizations Mar 26, 2020
@dhleong
Copy link
Owner

dhleong commented Mar 26, 2020

Oooh okay. I was looking at the pre-optimization files, I guess.

So what's happening is under :simple optimization, the style factory functions are declared as, eg:

yacwat_simple.styles.global_class_factory$=function(a,b){ /* ... etc */}

Under :none optimizations (dev mode), it looks like:

yacwat_simple.styles.global_class_factory$ = (function yacwat_simple$styles$global_class_factory$(style_name20047,params20048){
/* ... etc */
})

and in :advanced optimization, it's simply eg:

function p_(a,b){ /* ... etc. */ }

As background, to avoid having to embed a string with the style name (and consequently unnecessarily inflating the JS file in :advanced compilation mode) I'm using the name of the function that generates the style to uniquely identify the style across the app. This makes sense because in :none mode the name of the function is the fully-qualified clojure name, and in advanced it's a globally unique short name, so in development you can also easily see which element comes from which function.

Since I'm assuming that closure will generate a function with a name, the way :simple optimizations generates the function declaration javascript breaks everything. Hmm....

@dhleong dhleong self-assigned this Mar 26, 2020
@dhleong
Copy link
Owner

dhleong commented Mar 26, 2020

Hey so I just published version 1.0.4 which I believe fixes the issue. Please let me know how it works for you!

@sansarip
Copy link
Author

sansarip commented Mar 26, 2020

Huzzah! It works! Thanks for the speedy response and fix!

As an aside, I've been using this library successfully since its public debut, and it has been a joy to switch over to from css-modules and herb. Love the simplicity and flexibility it provides, and I've been a heavy advocate for it when the occasions arise in r/Clojure and at the workplace! Just commending you on your work :)

@dhleong
Copy link
Owner

dhleong commented Mar 26, 2020

Your welcome! Glad to hear it's working :)

Thanks so much! That means a lot. I started this because I was also a bit frustrated with the existing solutions, so I'm happy to see that others are enjoying it as much as I am :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants