cljsjs / packages

DEPRECATED: Javascript libraries packaged up with Google Closure externs
http://cljsjs.github.io
787 stars 816 forks source link

ReactRouter #46

Closed ghedamat closed 9 years ago

ghedamat commented 9 years ago

Hi,

I'm currently working on a simple wrapper for react-router to be used in reagent

I'm fairly new to the clojurescript environment but cljsjs looks like the best option to include this sort of dependency.

If you agree I'd love to try putting together the package. I might need a bit of direction though :)

thanks!

pandeiro commented 9 years ago

Hi @ghedamat, let's see what we can do.

From what I can see, react-router doesn't ship with a "global build" included in its repository. The javascript files in its dist folder assume a CommonJS/Webpack style build tool, and the global build is only available via something like bower.

The global build can be produced from the repo, but this too requires Webpack and Node.js.

Currently the boot-cljsjs tool has no specific means of either a) retrieving files via bower, nor b) executing a build script from the fileset. I'll @martinklepsch and @Deraen to see if they have any thoughts on the best way to go about this.

ghedamat commented 9 years ago

@pandeiro that would be amazing!

yeah, I'm currently manually pulling in the react-router.js build in my project and adding it to project.clj

https://github.com/ghedamat/reagent-react-router-demo/blob/master/project.clj#L51-L52

I have to apologize for my noobness here :( I should start looking more into the boot-cljsjs but the bower package simply points to https://github.com/rackt/react-router/blob/master/dist/react-router.js that ends up assigning a window.ReactRouter global object that exposes all the different components

I think I'm failing to understand why that would not work.

I'll try to read up more asap

thanks for all the help!

pandeiro commented 9 years ago

@ghedamat When I try to load the included source (dist/react-router.js), I get this error:

Uncaught
    LocationActions.PUSH
    __webpack_require__
    (anonymous function)
    __webpack_require__
    (anonymous function)
    __webpack_require__
    (anonymous function)
    (anonymous function)
    webpackUniversalModuleDefinition
    (anonymous function)
    (anonymous function)
    InjectedScript._evaluateOn
    InjectedScript._evaluateAndWrap
    InjectedScript.evaluate

TypeError: Cannot read property 'PropTypes' of undefined

Please let me know if you have better luck though. My understanding is that the build.sh script is doing some transformation to produce a global build from the Webpack version that ships in the repo.

ghedamat commented 9 years ago

@pandeiro you're absolutely right and I remembered wrong! sorry!

the version I've included in my build process is from cdnjs

https://cdnjs.cloudflare.com/ajax/libs/react-router/0.11.6/react-router.js which is what was included in the original lein template I started the project with :/

I'll have to look into where the bower version is coming from.

I'll try to compile react-router locally and report back sorry for pointing you in the wrong direction!

pandeiro commented 9 years ago

@ghedamat I saw that, too -- the cdnjs version is 0.11.6 as you said, while stable is 0.12.0. If all else fails we can just go with that (slightly older version)... but I think we may hit this issue again with other Webpack-based stuff in the React ecosystem. So it's worth coming up with a strategy IMO.

ghedamat commented 9 years ago

@pandeiro I looked at the react build process and got it running locally, the output of the build script is what is stored under the dist/ directory for the react-router repo which is what bower points when you user the bower package.

I updated my demo app to pull react-router from bower and it seems to be working just fine https://github.com/ghedamat/reagent-react-router-demo

I'm basically just using this file https://github.com/rackt/react-router/blob/master/dist/react-router.js (same one that I wrongly mentioned initially) and I don't get those errors.

If you want to test the demo app without bower you can simply pull the file in (you'll also have to locally lein install https://github.com/ghedamat/reagent-react-router)

if what I'm seeing proves to be true could we just have a cljs-boot script that simply pulls the dist file from github and avoid the build step?

pandeiro commented 9 years ago

could we just have a cljs-boot script that simply pulls the dist file from github and avoid the build step?

@ghedamat Well we could just do that (it would be great), but only once you test that the dist/ version (not the bower version) works just fine. "Basically" doesn't cut it here; it has to be the exact same file -- that's what cljsjs would deliver, after all.

ghedamat commented 9 years ago

@pandeiro what is on bower is what is here https://raw.githubusercontent.com/rackt/react-router/master/dist/react-router.js bower simply points you to the git repo (and likely does some caching)

To double check, just downloaded the file linked above and did a diff and they're the same file.

the part that still confuses me is the the externs, do we need them?

let me know how we should proceed and thanks again for all the help!

pandeiro commented 9 years ago

Yikes, okay, so it is. Still I can't access the ReactRouter object with that source file.

Re: externs, I would use this (if that object were available): http://www.dotnetwise.com/Code/Externs/

Edit: BTW you can use this to try to load the git version into the Externs generator: https://rawgit.com/rackt/react-router/master/dist/react-router.js

pandeiro commented 9 years ago

OK, the react-router lib loads fine if React is already loaded. Can't believe I didn't catch this! Sorry!

@ghedamat If you want to try to package it, go for it! Otherwise I can do it when I have a sec.

ghedamat commented 9 years ago

thanks @pandeiro I'll give it a shot, I'm curious :) I'll for this repo, try to add the package and report back here

thanks again

ghedamat commented 9 years ago

@pandeiro thanks for the pointers, the extern generator probably can't help due to the fact that it's a webpack module with an IIFE, that said I think I was able to get the externs just by looking at the exports on the toplevel module. https://github.com/rackt/react-router/blob/master/dist/react-router.js#L57-L77 I'll try to open a pr soon, currenlty trying to convince boot to compile for me :D

pandeiro commented 9 years ago

@ghedamat I was able to generate an extern with that page by first loading React and then loading React Router. I'll paste here for convenience:

var ReactRouter = {
    "DefaultRoute": function () {},
    "Link": function () {},
    "NotFoundRoute": function () {},
    "Redirect": function () {},
    "Route": function () {},
    "RouteHandler": function () {},
    "HashLocation": {
        "addChangeListener": function () {},
        "removeChangeListener": function () {},
        "push": function () {},
        "replace": function () {},
        "pop": function () {},
        "getCurrentPath": function () {},
        "toString": function () {}
    },
    "HistoryLocation": {
        "addChangeListener": function () {},
        "removeChangeListener": function () {},
        "push": function () {},
        "replace": function () {},
        "pop": function () {},
        "getCurrentPath": function () {},
        "toString": function () {}
    },
    "RefreshLocation": {
        "push": function () {},
        "replace": function () {},
        "pop": function () {},
        "getCurrentPath": function () {},
        "toString": function () {}
    },
    "ImitateBrowserBehavior": {
        "updateScrollPosition": function () {}
    },
    "ScrollToTopBehavior": {
        "updateScrollPosition": function () {}
    },
    "History": {
        "length": {},
        "back": function () {}
    },
    "Navigation": {
        "contextTypes": {
            "makePath": function () {},
            "makeHref": function () {},
            "transitionTo": function () {},
            "replaceWith": function () {},
            "goBack": function () {}
        },
        "makePath": function () {},
        "makeHref": function () {},
        "transitionTo": function () {},
        "replaceWith": function () {},
        "goBack": function () {}
    },
    "RouteHandlerMixin": {
        "contextTypes": {
            "getRouteAtDepth": function () {},
            "setRouteComponentAtDepth": function () {},
            "routeHandlers": function () {}
        },
        "childContextTypes": {
            "routeHandlers": function () {}
        },
        "getChildContext": function () {},
        "componentDidMount": function () {},
        "componentDidUpdate": function () {},
        "componentWillUnmount": function () {},
        "_updateRouteComponent": function () {},
        "getRouteDepth": function () {},
        "createChildRouteHandler": function () {}
    },
    "State": {
        "contextTypes": {
            "getCurrentPath": function () {},
            "getCurrentRoutes": function () {},
            "getCurrentPathname": function () {},
            "getCurrentParams": function () {},
            "getCurrentQuery": function () {},
            "isActive": function () {}
        },
        "getPath": function () {},
        "getRoutes": function () {},
        "getPathname": function () {},
        "getParams": function () {},
        "getQuery": function () {},
        "isActive": function () {}
    },
    "create": function () {},
    "run": function () {}
}
ghedamat commented 9 years ago

oh! amazing! it makes sense actually

ghedamat commented 9 years ago

@pandeiro can I also ask you a suggestion for testing this locally?

I'm trying with boot package build-jar that should push it to my local repo,

but having little luck so far in loading it later from my lib.. I'm likely doing it wrong :D

pandeiro commented 9 years ago

@ghedamat That is the right procedure. First, you might just do boot package and inspect the target directory to see that the expected files were output. (You could also open up the jar in .m2/ to see this.)

If the files are where they should be, but the global ReactRouter variable is not present in pages that load namespaces which require it (eg (ns myapp.core (:require cljsjs.react-router))), then we have a problem.

ghedamat commented 9 years ago

my build task is currenlty failing because the namespace cljsjs.react-router is not found..

also @pandeiro do you happen to be on irc? :D I could jump on there if it's easier

pandeiro commented 9 years ago

Sure @ghedamat, let's chat there.

ghedamat commented 9 years ago

I'd say we can close this :) thanks again!