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 externs.js and the exported symbols #216

Closed
wants to merge 1 commit into from

Conversation

thheller
Copy link
Contributor

The datascript/externs.js is written incorrectly.

Including the externs via deps.cljs means that CLJS projects can never rename them as well which isn't useful. CLJS projects that want to use this would be better off without the externs/exports.

You should consider only using the externs/exports when building the JS lib.

@tonsky
Copy link
Owner

tonsky commented Apr 11, 2017

Sure, but DataScript implementation depends on datom fields not being renamed. I have to include those. As for entity, what I was doing is aimed for JS only, that’s right.

@zarkone
Copy link

zarkone commented May 19, 2018

Not sure if that related but I also had problems with datascript, compiling with shadow-cljs.

For example, this code works without optimizations, but doesn't work with :advanced enabled, i.e. in :realease mode:

(d/q '[:find [?atom ...] :in $ ?url
                         :where [?e :url ?url]
                                [?e :atom ?atom]]
                   @db url)

I've solved it without patching anything by adding contents of datascript/externs.js to my manual-externs.js file I already had for externs:

Related part of shadow-cljs.edn:

:release {:compiler-options {:externs ["manual-externs.js"]
                             :optimizations :advanced}}

@tonsky
Copy link
Owner

tonsky commented Nov 29, 2018

@zarkone are you sure it’s datascript problem? maybe your build just doesn’t know how to lookup externs served with libraries?

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