You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A pull request was submitted in relation to issue 285 to add tests validating that salience works in ClojureScript. In this PR, there was this usage of defsession:
The point of having defsession in ClojureScript is that due to the lack of eval in ClojureScript, which is used heavily by mk-session we can't generate sessions at runtime in ClojureScript as we can in Clojure. However, because the rule sources work at compile time and only the options needed to fall through to runtime evaluation, and the defsession macro was organized in a way that inadvertently facilitated this, this actually worked.
I didn't want to derail adding the tests with this larger issue, but I'd like to consider a couple options here. This was also discussed in a comment on the PR at #288 (comment).
Should we create a macro that doesn't create a definition for the session? Basically it would just be a macro that has this same behavior, but intentionally. If we add this, should it be for internal test use only or a public API?
There has been some work to add eval to ClojureScript; perhaps this is mature enough that we could use it in Clara's tests.
Other options to consider?
The text was updated successfully, but these errors were encountered:
A pull request was submitted in relation to issue 285 to add tests validating that salience works in ClojureScript. In this PR, there was this usage of defsession:
The point of having defsession in ClojureScript is that due to the lack of eval in ClojureScript, which is used heavily by mk-session we can't generate sessions at runtime in ClojureScript as we can in Clojure. However, because the rule sources work at compile time and only the options needed to fall through to runtime evaluation, and the defsession macro was organized in a way that inadvertently facilitated this, this actually worked.
I didn't want to derail adding the tests with this larger issue, but I'd like to consider a couple options here. This was also discussed in a comment on the PR at #288 (comment).
The text was updated successfully, but these errors were encountered: