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

Investigate using :npm-deps to specify dependencies for Shadow CLJS users #188

Closed
danielcompton opened this issue Apr 4, 2018 · 6 comments · Fixed by #201
Closed

Investigate using :npm-deps to specify dependencies for Shadow CLJS users #188

danielcompton opened this issue Apr 4, 2018 · 6 comments · Fixed by #201
Labels
Milestone

Comments

@danielcompton
Copy link
Contributor

#180 (comment)

Don't know how you feel about adding the npm deps to :npm-deps in a deps.cljs for re-frame-10x so they don't have to be added manually. I believe however this messes with standard CLJS when still using CLJSJS.

We should look into how this works and what we can do to make things easier for Shadow users.

@danielcompton danielcompton changed the title Investigate using :npm-deps to specify dependencies Investigate using :npm-deps to specify dependencies for Shadow CLJS users Apr 4, 2018
@Deraen
Copy link
Contributor

Deraen commented Apr 26, 2018

No need to use :npm-deps here.

If the Cljsjs packages used here (react-filp-move and react-highlight) are updated to use :global-exports, and provide npm-style names, they can be required in the same manner as Cljs namespaces or Node modules:

(:require [react-flip-move :as react-flip-move])

The the same code will work with Cljsjs packages, ClojureScript compiler with Node modules and with Shadow-cljs.

E.g create-react-class provides simple example of Cljsjs package which uses global-exports and provides the npm-style name: https://github.com/cljsjs/packages/blob/master/create-react-class/build.boot#L25-L27 (and for backwards comptilibity it also provides the old-style cljsjs.* name). Cljsjs wiki provides some more information on this: https://github.com/cljsjs/packages/wiki/Unified-module-names-and-global-exports

Reagent 0.8.0 already uses this to access React, so that will work with all environments.

@thheller
Copy link
Contributor

@Deraen :npm-deps would be useful since shadow-cljs would then know which npm packages re-frame-10x requires. It is only about triggering the install of those :npm-deps not about making the compiler use them.

Not sure if that would interfere with standard CLJS compilation in any way though.

@Deraen
Copy link
Contributor

Deraen commented Apr 26, 2018

Should be okay. ClojureScript compiler only uses deps.cljs :npm-deps when :install-deps true is set, so it shouldn't affect use with default settings. I'll have PR with updated Cljsjs deps soon, that should make it easy to support all envs.

@danielcompton
Copy link
Contributor Author

Is there more required to properly fix this?

@danielcompton danielcompton reopened this Feb 19, 2019
@talgiat
Copy link

talgiat commented Apr 29, 2019

We are using shadow-cljs with newer react versions and when you build the project with shadow it tries to npm install the versions in re-frame-10x deps.cljs. I couldn't find a way to prevent that, because shadow does not use the :install-deps compiler option (at least according to the docs)

@superstructor
Copy link
Contributor

@talgiat Thanks for reporting. This has been fixed on master and will be part of the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants