Fork me on GitHub
#cljsrn
<
2017-07-14
>
carocad08:07:43

hey guys, I did some work with @drapanjanas to improve the dev experience of re-natal. We introduced a new command infer-components to avoid having to manually lookup the components that you just introduced in your source code. It is a simple improvement but hopefully a useful one ๐Ÿ™‚ The command is available on the latest re-natal v0.5.0

pesterhazy08:07:25

@carocad nice! how does that work? Is it a replacement for use-component?

pesterhazy08:07:06

as a side note, use-component is not a great name for the task, given that it can (must!) be used for npm dependencies that aren't react components at all, like moment.js

carocad08:07:33

@pesterhazy yes. It is intended to replace it. It scans all cljs files and extracts js/require statements from it

pesterhazy08:07:00

interesting!

pesterhazy08:07:27

will it still be possible to hook things up manually?

carocad08:07:19

currently it is not possible but it should be straight forward to introduce a flag for custom requires. It is a simple regex that we run ๐Ÿ˜‰

pesterhazy08:07:41

I think that would be good

pesterhazy08:07:21

also I imagine a regex is not foolproof (imaging you have a js/require in a comment) so it would be great if it was possible to override it

pesterhazy08:07:44

or switch to updating .re-natal manually as we do currently

carocad08:07:51

@pesterhazy yes that's correct. That is the reason why currently both use-component and infer-component coexists. We couldnt find a one solution fits all so we decided to go with both ways and test it out.

pesterhazy08:07:08

sounds good to me

carocad08:07:23

infer-components also informs you of the diff of components so that you are aware if something was added that shouldnt

pesterhazy08:07:32

@drapanjanas did you fix the problems with reagent 0.7.0?

drapanjanas08:07:36

No not yet, will try that this weekend

pesterhazy08:07:52

ok cool, happy to help debug issues

drapanjanas08:07:41

You mentioned additional lib required, but i thouht it should be a transitive dependecy of reagent, but i need to have closer look

pesterhazy08:07:19

well in cljsrn-land we get our react from npm, no? so I think we should take https://www.npmjs.com/package/create-react-class from npm too

pesterhazy09:07:06

we'd also need a substitute namespace for cljsjs/create-react-class

pesterhazy09:07:54

or do you think it's possible to take create-react-class from cljsjs and it'll just pick up the existing js/window.React?

carocad09:07:54

@pesterhazy is this related to an error saying react module should be required before createClass? I just updated my dependencies and now my app is broken ๐Ÿ˜•

pesterhazy09:07:16

quite possible

carocad09:07:46

do you know the solution to it? I dont get the problem from the stacktrace

carocad09:07:41

it is somehow pointing me to the figwheel-bridge but there react is required first

pesterhazy09:07:31

in my experience all re-natal stack traces point to the figwheel-bridge so that's probably not it

pesterhazy09:07:42

what did you change?

carocad09:07:08

I updated the dependencies of the project. Clj, cljs, reagent ...

pesterhazy09:07:37

did you upgrade reagent to 0.7.0?

pesterhazy09:07:20

if yes that's probably the culprit. I'd start by downgrading that to verify the hypothesis

carocad09:07:08

yes I did. I will try that

carocad09:07:08

@pesterhazy as always you were right ๐Ÿ™‚ genious !

carocad09:07:19

downgrading reagent to 0.6.0 solved the issue

carocad09:07:14

just in case it might be useful to somebody. The following dependencies work together on my project:

[org.clojure/clojure "1.9.0-alpha17"]
[org.clojure/clojurescript "1.9.671"]
[reagent "0.6.0" :exclusions [cljsjs/react cljsjs/react-dom cljsjs/react-dom-server]]
[re-frame "0.9.4"]]
:plugins [[lein-cljsbuild "1.1.6"]

pesterhazy09:07:06

0.6.2 should work as well

pesterhazy09:07:21

nonetheless we should look into the reagent 0.7.0 problems

carocad09:07:15

well from the way you say it, it seems as it is a known problem. Do you know what is it? or is there already a proposal of how to fix it? I am happy to help with testing if you need ๐Ÿ™‚

pesterhazy10:07:57

if you create an issue (on re-natal or reagent) with a minimal repro, I'll take a look

carocad10:07:15

ok let me check what I can come up with ๐Ÿ™‚

carocad10:07:51

btw there createClasss thing is mentioned on the reagent changelog. Have you seen that?

carocad10:07:37

it explicitedly mentions having to deal with createClass but since I am not so familiar with the figwheel-bridge I dont know exactly where to poke

pesterhazy10:07:05

i think what you'll need to do is 1. add create-class to package.json and 2. add a shim namspace to prevent this from being loaded via cljsjs and 3. make sure reagent finds the right create-class

pesterhazy10:07:17

but not 100% that's the best way to go

carocad10:07:09

@drapanjanas I just ran re-natal init foo and the project.clj now contains #_($PLATFORM_CLEAN$) DEV_PROFILES and similars. Is this intended? or is this a bug?

pesterhazy10:07:49

i have that in mine too and was wondering what that was about

drapanjanas10:07:23

It is intended, these tags are used in feature to add windows platforms

drapanjanas10:07:36

You can clean those if you will not add windows platform for your app

drapanjanas10:07:27

re-natal add-platform uses that

drapanjanas10:07:07

To add new build configs using regex

carocad10:07:56

ah ok :thumbsup::skin-tone-4:

carocad10:07:18

btw, not sure if you intended this to happen but in my project.clj the #_($DEV_PROFILES$ closes over the compled :prod profile and :repl-options. Probably due to parinfer changing the closing parenthesis

drapanjanas10:07:08

I noticed that too, parinfer seem to have trouble with those- i removed those in my build

carocad11:07:39

wouldnt it be better if you put those both DEV_PROFILE and PROD_PROFILE as comments? that way parinfer wouldnt mess those up

drapanjanas19:07:53

Yes, that might work too

carocad11:07:25

@pesterhazy you can find a reproducible step by step issue with some extra information here: https://github.com/drapanjanas/re-natal/issues/122

carocad11:07:28

hope it helps ๐Ÿ™‚

pesterhazy12:07:20

@carocad I've attached a patch, can you see if it works for you?

carocad13:07:37

@pesterhazy do I need to run npm install on create-react-class?

pesterhazy13:07:37

good question - AFAICT it's a dependency of react-native 46 so it should be installed by default

pesterhazy13:07:54

as to whether it'l stay that way, not sure

carocad13:07:00

yeah I just checked it and it is so no problem requiring it

carocad13:07:36

@pesterhazy it works ๐Ÿ™‚

drapanjanas18:07:41

@pesterhazy @carocad thanks just bumped reagent in re-natal, it works ๐Ÿ™‚

pesterhazy18:07:39

Is it worth documenting the min react version?

pesterhazy18:07:19

Not sure how often people update their re-natal

drapanjanas18:07:37

what about advanced build? Iโ€™d like to add lein advanced-build in addition to lein prod-build which will stay as is

drapanjanas18:07:06

Yes, need to think about it before releasing

pesterhazy18:07:09

Sounds like a good idea to me!

pesterhazy18:07:29

We'll need externs in that case

pesterhazy18:07:06

Or is that taken care of by the recent cljs compiler changes?

pesterhazy18:07:05

I agree that adv compilation is more interesting now that mfikes reported faster startup times

drapanjanas18:07:49

and I heard it works-around some packager problems

carocad18:07:39

for what is worth I am using cljs 1.9.671 with infer-externs and warn-on-infer. I havent done an advance compilation yet though. I will let you know once I have some data

mfikes18:07:40

The holy grail would be to actually include React in the :advanced compilation

mfikes18:07:43

Secondarily, it would be nice to be able to use :npm-deps so we can move stuff into the ns forms

pesterhazy18:07:02

It would be nice to have a CI job that runs an app on iOS

pesterhazy18:07:42

Yeah we do that, circle works well for that

pesterhazy18:07:13

I meant it would be cool to have a job that verifies re-natal

drapanjanas19:07:04

I agree, now I test it manually..

drapanjanas19:07:36

but never did that, so need to understand how

pesterhazy19:07:06

I can help set it up if that's helpful

drapanjanas19:07:20

is it in github do you need some rights?

drapanjanas19:07:23

@mfikes did not quite understand about the adding React in :advanced.

mfikes19:07:31

So @drapanjanas my speculation is that we are not passing the React JavaScript through Google Closure when we set the ClojureScrpt compiler to be in :advanced mode.

drapanjanas19:07:32

yes, React and ReactNative is outside of our compilation totally

drapanjanas19:07:31

if it would be possible to add those, need to remove the same from bundle produced by packager

mfikes19:07:49

If we can get things to instead flow through :npm-deps then we could get Closure DCE, etc.. But the React Native package manager makes for a challenge

mfikes19:07:38

We just need Antรณnio Monteiro to get sucked into building a React Native app, and I bet he will figure out good stuff ๐Ÿ™‚

drapanjanas19:07:49

true ๐Ÿ™‚

carocad19:07:20

@mfikes have you written somewhere about your attempts to get it working?

mfikes19:07:46

@carocad No, because I did not get very far at all. Just try adding React to :npm-deps following the stuff here https://anmonteiro.com/2017/03/requiring-node-js-modules-from-clojurescript-namespaces/

carocad19:07:45

@mfikes interesting. I was trying a different approach. I tried using haul to avoid the rn packager and get it to recognize goog.require statements. I failed though since haul doesnt transpile source files. I am trying to wrap my head around this post to see I can use it to make it work: https://clojurescript.org/guides/javascript-modules

carocad19:07:11

there is a part about react and react/dom so I figured it might work

mfikes19:07:50

If you can get things to work the :npm-deps way, then another cool thing is you can use string-based requires in your ns forms, which may make it more โ€œnaturalโ€ to use React Native components