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

Fix macro typehint #145

Merged
merged 1 commit into from
Sep 18, 2023
Merged

Fix macro typehint #145

merged 1 commit into from
Sep 18, 2023

Conversation

aiba
Copy link
Contributor

@aiba aiba commented Sep 14, 2023

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

@lilactown
Copy link
Owner

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

@rome-user
Copy link
Contributor

rome-user commented Sep 17, 2023

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
Copy link
Contributor Author

aiba commented Sep 18, 2023

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
Copy link
Owner

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.

@lilactown lilactown merged commit ecaafdb into lilactown:master Sep 18, 2023
1 check passed
lilactown pushed a commit that referenced this pull request Jun 10, 2024
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

Successfully merging this pull request may close these issues.

"Provider" property access getting mangled by :optimizations :advanced
3 participants