Fork me on GitHub
#cljs-dev
<
2017-05-29
>
thheller18:05:51

curious to hear some thoughts on the subject, happy to discuss my chosen implementation

juhoteperi19:05:14

Foreign-libs can be used without re-packaged NPM packages.

juhoteperi19:05:06

And foreign-libs can already be used to provide react and without module-processing it should directly use the exported React object instead of global React object

juhoteperi19:05:14

If I remember correctly, one thing foreign-libs are missing is support for example require('react-dom/server')

thheller19:05:23

@juhoteperi yeah :foreign-libs can probably be modified a bit a make this work. most CLJSJS packages are however not set up that way.

juhoteperi19:05:25

I'd compare this against :npm-deps + foreign-libs more than against Cljsjs

thheller19:05:26

as you said 'react-dom/server' or even "react-dom" -> ReactDOM

juhoteperi19:05:12

But yeah, I guess using strings for names will be the easiest way to use the same names as in JS

thheller19:05:22

didn’t want to compare it to anything, in some sense it is just syntax sugar to what you can currently do

thheller19:05:31

just noticed that I currently have to work around CLJSJS a bit but that is quite common nowadays I guess

thheller19:05:43

react-native folks seem to be doing it also

thheller19:05:34

saw some templates that provide empty cljsjs.react namespaces and the like

juhoteperi19:05:37

Yeah, I think we need to either some way for libraries to be agnostic to how the deps are provided, or have a common naming convention Cljsjs and all the other ways can use

thheller19:05:41

it works nicely for node.js code but the extra build step for browser targets isn’t as nice as :foreign-libs + CLJSJS

thheller19:05:49

works nicely if you are using a JS build tool anyways though (ie. webpack, create-react-app, create-react-native-app, etc)

juhoteperi19:05:22

I'm just trying to use :npm-deps with Reagent and I think the current Reagent code won't work unless I create a shim NS for cljsjs.react which defines js/React and exports the functions requires by Reagent to that object 😕

thheller19:05:50

yeah exactly

juhoteperi19:05:35

Huh, and is React from :npm-deps even supposed to work with just Closure module-processing, it is trying to call process & other strange things

juhoteperi19:05:09

Though it is possible that that is caused by my Boot tooling trying to load JS files that are not really used and should not be evaluated

juhoteperi19:05:48

If libraries continue to access libraries through global window props, it is going to be hard for the libraries to support multiple ways provide the deps

thheller19:05:23

my implementation works by mapping the (:require ["react" :as react]) to a pseudo goog namespace

thheller19:05:48

so you get goog.provide("shadow.npm.react"); shadow.npm.react = require("react")

juhoteperi19:05:21

That is quite similar to how npm-deps works

thheller19:05:23

and the "react” just gets replaced by that alias, so it is basically (:require [shadow.npm.react :as react])

juhoteperi19:05:43

dynamic names are just a mess for npm-deps: goog.addDependency("../node_modules/react-dom/index.js", ['module$home$juho$Source$saapas$node_modules$react_dom$index'], ['module$home$juho$Source$saapas$node_modules$react_dom$lib$ReactDOM']);

thheller19:05:23

yeah sort of … can’t really compare this to :npm-deps since the code doesn’t actually go through closure afterwards

thheller19:05:46

but it is the same idea yeah

shaunlebron22:05:24

👋 would it be good to submit a patch to allow (= ^string foo "value") to compile as (identical? foo "value") to allow simpler DCE? like the ^boolean type hints allow unchecked-if