search

lilactown / helix

A simple, easy to use library for React development in ClojureScript.
Eclipse Public License 2.0
624 stars 52 forks source link

Fix macro typehint #145

Closed aiba closed 11 months ago

aiba commented 11 months ago

Unfortunately, #109 did not fix #103. This is a correct fix for #103.

lilactown commented 11 months ago

Hmm, OK. Is there a test we can add for this?

rome-user commented 11 months ago

I think the best we can do is write an example context provider in a test that is compiled with advanced optimizations. In general, its a good idea to set up tests that go through advanced optimizations. I do this at my job to help prevent obscure bugs in production that do not manifest in developer build

aiba commented 11 months ago

Adding to what rome-user said, yeah the way to test this is to do an advanced compilation of some code that uses the provider macro. When the bug is present, you will see a warning that looks something like:

------ WARNING #1 - :infer-warning ---------------------------------------------
Cannot infer target type in expression (. ctx__48164__auto__ -Provider)

When the bug is not present, there will be no warning in the compiler output.

lilactown commented 11 months ago

Okay, I'll accept this as-is and think about how I'd like to test this.

This might change based on the CLJS compiler as well, so I think it makes sense to make a best effort but not guarantee. The fix is easy to implement in projects if necessary and the compiler ought to warn if issues with this approach occur.