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

Move ClojureScript dep to profiles #65

Closed
bbatsov opened this issue Jul 17, 2019 · 5 comments · Fixed by #143
Closed

Move ClojureScript dep to profiles #65

bbatsov opened this issue Jul 17, 2019 · 5 comments · Fixed by #143
Assignees
Labels
good first issue Good for newcomers

Comments

@bbatsov
Copy link
Member

bbatsov commented Jul 17, 2019

Run now we've hardcoded the dep, which is bad for end users. It should go to all profiles the way we've done for other projects (e.g. https://github.com/clojure-emacs/cider-nrepl/blob/master/project.clj).

@arichiardi The current code will work with older ClojureScript releases, right?

@SevereOverfl0w
Copy link
Collaborator

I don't know the state of the code, but can we ensure that all the code works without cljs loaded, lest we end up with the same problem we have at piggieback.

@bbatsov
Copy link
Member Author

bbatsov commented Jul 20, 2019

Well, that would be ideal, but it might be a bit harder here. When I agreed to baking the ClojureScript support in Orchard I didn't now many deps ClojureScript was pulling in (I assumed it didn't have any runtime deps). While that's not an issue for cider-nrepl, which obfuscates the deps, it will be an issue for everyone using orchard directly.

@SevereOverfl0w
Copy link
Collaborator

The clojurescript dependency cannot be obfuscated. Otherwise you won't be accessing the right clojurescript state (via the compiler env)

@bbatsov
Copy link
Member Author

bbatsov commented Aug 16, 2019

Ah, yeah. Silly me. Well, I guess we'll figure something out.

@SevereOverfl0w
Copy link
Collaborator

Piggieback solved this with a "shim" to only load cljs code when cljs was available.

@bbatsov bbatsov added the good first issue Good for newcomers label Dec 22, 2021
vemv added a commit that referenced this issue Jan 8, 2022
vemv added a commit that referenced this issue Jan 8, 2022
vemv added a commit that referenced this issue Jan 9, 2022
vemv added a commit that referenced this issue Jan 9, 2022
vemv added a commit that referenced this issue Jan 9, 2022
vemv added a commit that referenced this issue Jan 9, 2022
@vemv vemv closed this as completed in #143 Jan 9, 2022
vemv added a commit that referenced this issue Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants