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

Fully qualify symbols used in cljs metadata schemas #660

Merged

Conversation

dvingo
Copy link
Contributor

@dvingo dvingo commented Mar 8, 2022

This PR ensures all vars used in cljs schemas are fully qualified. This is necessary because the namespace where collect! is invoked will be the one where the generated code from the macro is produced, which likely won't have those vars available.

This should fix the issue #659

@ikitommi ikitommi merged commit da7a19c into metosin:master Mar 8, 2022
@ikitommi
Copy link
Member

ikitommi commented Mar 8, 2022

Thank You.

@kovasap
Copy link

kovasap commented Mar 16, 2022

One potential issue with this PR (not sure if it's intended or not) is that predicate functions like float? and int? can't be used in schemas - when I try, I get an error that my-namespace/float? doesn't exist.

@dvingo
Copy link
Contributor Author

dvingo commented Mar 17, 2022

Thanks for thoroughly testing this @kovasap I think we can check if the symbols are in cljs.core or not only qualify the ones that are not. I should have some time this weekend to look into this.

@dvingo dvingo deleted the fix-cljs-instrument-resolve-symbols branch May 5, 2023 16:12
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.

3 participants