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

Marty v0.9 #137

Merged
merged 0 commits into from
Mar 24, 2015
Merged

Marty v0.9 #137

merged 0 commits into from
Mar 24, 2015

Conversation

jhollingworth
Copy link
Contributor

Improvements

Bugs

Tasks

@dariocravero
Copy link
Contributor

This looks like a very good step forward :). Need any help?
Also, should we make marty ES6-proof using something like Babel (previously 6to5) if no native ES6 support is available?

@jhollingworth
Copy link
Contributor Author

Hey, yeah would love some help 😄 Would you be interested in picking up #132? I was playing around with it the other day but got distracted...

Completely agree on Babel, I started re-writing everything to be ES6 classes the other day. It makes isomorphisim a whole load easier if everything's instantiatable. Things like Marty.createStore will still be supported but I'm adding Marty.register(SomeStore) for ES6 classes. Hopefully will be done by next week...

@FoxxMD
Copy link

FoxxMD commented Feb 19, 2015

I'd be happy to help test when you need it 👍

@dariocravero
Copy link
Contributor

Good stuff! :) Will try to tackle #132 over the next few days. As for Babel, #138 might be a good a start. :) <3 to see an es6 branch already! :)

@Dakuan
Copy link

Dakuan commented Feb 19, 2015

This looks awesome - we are really hoping to use marty for new project but lack of server rending is a dealbreaker. Can pitch in some support testing things.

@dariocravero
Copy link
Contributor

@jhollingworth quick one, have you considered migrating the testing suite to jest?

@jhollingworth
Copy link
Contributor Author

Jest is cool however it would require converting all tests from mocha to jasmine. Also, I don't think it integrates well with SauceLabs (thats based on a quick google so could be wrong).

@jhollingworth jhollingworth force-pushed the v0.9 branch 2 times, most recently from 7d878c1 to ccde31e Compare February 20, 2015 21:02
@dariocravero
Copy link
Contributor

@jhollingworth on mocha/jest: jestjs/jest#139

@Dakuan
Copy link

Dakuan commented Feb 23, 2015

Just had a quick look at this to have a play with the server rendering but having some issues with webpack:

First I got this sort of thing:

ERROR in ./~/marty/lib/actionCreators/actionCreators.js
Module parse failed: /Users/Dom/git/anu/node_modules/marty/lib/actionCreators/actionCreators.js Line 15: Unexpected reserved word
You may need an appropriate loader to handle this file type.
| }
|
| class ActionCreators {

which I'm guessing is the es6 stuff. I added babel-loader to my webpack config:

{
    test: /\.(es6|jsx|js)$/,
    loader: 'babel-loader'
}

which makes the errors go away - but breaks the webpack-dev-server. I see this in the browser console:

 Uncaught TypeError: Cannot use 'in' operator to search for 'document' in undefined

@jhollingworth
Copy link
Contributor Author

@Dakuan does it tell you what line that error is thrown on?

@dariocravero
Copy link
Contributor

@Dakuan can you paste your full webpack.config.js?

@Dakuan
Copy link

Dakuan commented Feb 25, 2015

@jhollingworth @dariocravero

managed to fix with this webpack config:

{
            test: /\.(jsx|es6|js)$/,
            loader: 'babel-loader',
            include: /(?=src)|(?=node_modules\/marty)/
        }

and by disabling the marty chrome plugin

@jhollingworth
Copy link
Contributor Author

@Dakuan awesome! I going to start on a webpack demo soon so this will be useful 😄

@jhollingworth
Copy link
Contributor Author

Good point about Marty Developer Tools, have added it to the list of things to look at. Thanks

@Dakuan
Copy link

Dakuan commented Feb 25, 2015

Yep! Some progress but not out of the woods yet. When trying to require (on the server) a file that is connected with the marty app eg some kind of entry point like app.jsx or routes.jsx you get an error as node wont like the class keyword being out of place.

A natural thing to do is to try and use the babel require polyfill from here https://babeljs.io/docs/usage/require/ . This still won't work yet as marty is ignored by babel due to it being in node_modules. That's easily fixed by adding the {ignore: true} flag to the babel config. It's dog slow first time - hopefully whitelisting marty rather than just allowing all the npm modules to be parsed will help, but it's not working as documented at the moment.

Anyway, that spins for a while and then we get:

-> node src/server/index.js

/Users/Dom/git/anu/node_modules/marty/node_modules/underscore/underscore.js:1536
}.call(this));
  ^
TypeError: Cannot read property '_' of undefined
    at /Users/Dom/git/anu/node_modules/marty/node_modules/underscore/underscore.js:15:32
    at Object.<anonymous> (/Users/Dom/git/anu/node_modules/marty/node_modules/underscore/underscore.js:1536:3)
    at Module._compile (module.js:456:26)
    at normalLoader (/Users/Dom/git/anu/node_modules/babel/lib/babel/api/register/node.js:102:5)
    at Object.require.extensions.(anonymous function) [as .js] (/Users/Dom/git/anu/node_modules/babel/lib/babel/api/register/node.js:115:7)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at Object.<anonymous> (/Users/Dom/git/anu/node_modules/marty/index.js:4:9)

Here is how I get babel running:

// index.js 

require("babel/register")({
    ignore: false
});

var app = require('./app');

// 4130 is a steel alloy. It's what you make BMX frames out of.
app.listen(process.env.PORT || 4130, function() {
    console.log('listening on *:4130');
});

@Dakuan
Copy link

Dakuan commented Feb 25, 2015

I should add that babel appears to be otherwise working, this test es6 class works as expected:

class Thing {
    log () {
        console.log('mew');
    }
}


module.exports = Thing;

@dariocravero
Copy link
Contributor

I was about to suggest babel should work as I was reading the thread on my email @Dakuan and then saw your last comment :).
You can always pipe your code thorugh babel-node; iojs might also be a better target than node going forward.

@Dakuan
Copy link

Dakuan commented Feb 26, 2015

Which package do you mean? Can't seem to find it here: https://www.npmjs.com/search?q=babel-node

@dariocravero
Copy link
Contributor

It's a binary that babel installs

@jhollingworth
Copy link
Contributor Author

It might be easier if node consumes the built files rather than having to get people to add shims or use different JS environments. Although that has the downside of making it slightly more painful to debug

@Dakuan
Copy link

Dakuan commented Feb 26, 2015

Yeah I tried requiring dist/marty but I got a babel error:

require("babel/register")({
    ignore: false
});

var app = require('./app');

gives:

/Users/Dom/git/anu/node_modules/marty/dist/marty.js:3241
global._babelPolyfill = true;
^
Error: only one instance of babel/polyfill is allowed
    at Object.<anonymous> (/Users/Dom/git/anu/node_modules/marty/dist/marty.js:3241:1)
    at Object.core-js/shim (/Users/Dom/git/anu/node_modules/marty/dist/marty.js:3246:66)
    at s (/Users/Dom/git/anu/node_modules/marty/dist/marty.js:1:637)
    at /Users/Dom/git/anu/node_modules/marty/dist/marty.js:1:679
    at Object.../../polyfill (/Users/Dom/git/anu/node_modules/marty/dist/marty.js:3234:30)
    at s (/Users/Dom/git/anu/node_modules/marty/dist/marty.js:1:637)
    at /Users/Dom/git/anu/node_modules/marty/dist/marty.js:1:679
    at Object../lib/babel/api/register/node (/Users/Dom/git/anu/node_modules/marty/dist/marty.js:5690:4)
    at s (/Users/Dom/git/anu/node_modules/marty/dist/marty.js:1:637)
    at /Users/Dom/git/anu/node_modules/marty/dist/marty.js:1:679

and without babel:

// require("babel/register")({
//     ignore: false
// });

var app = require('./app');

gives:

 -> node src/server/index.js

/Users/Dom/git/anu/node_modules/marty/dist/marty.js:7344
  if (self.fetch) {
      ^
ReferenceError: self is not defined
    at /Users/Dom/git/anu/node_modules/marty/dist/marty.js:7344:7
    at Object.<anonymous> (/Users/Dom/git/anu/node_modules/marty/dist/marty.js:7664:3)
    at s (/Users/Dom/git/anu/node_modules/marty/dist/marty.js:1:639)
    at /Users/Dom/git/anu/node_modules/marty/dist/marty.js:1:690
    at Object.whatwg-fetch (/Users/Dom/git/anu/node_modules/marty/dist/marty.js:7667:1)
    at s (/Users/Dom/git/anu/node_modules/marty/dist/marty.js:1:639)
    at /Users/Dom/git/anu/node_modules/marty/dist/marty.js:1:690
    at Object.../../http/hooks/parseJSON (/Users/Dom/git/anu/node_modules/marty/dist/marty.js:1762:1)
    at s (/Users/Dom/git/anu/node_modules/marty/dist/marty.js:1:639)
    at /Users/Dom/git/anu/node_modules/marty/dist/marty.js:1:690

@Dakuan
Copy link

Dakuan commented Feb 26, 2015

I'm wondering if es6 is more trouble than its worth for a lib that wants to be able to run anywhere. 😿

@jhollingworth
Copy link
Contributor Author

I've thought the same a few times recently... Once Marty.Component is done I'm going to focus on simplifying getting it to work on node. Hopefully this will be much better next week!

Thanks for all you're help trying this out, very much appreciated!

@oliverwoodings
Copy link
Contributor

@jhollingworth could we add another dist output that is precompiled into ES5? By default marty could be es5, but if you want to use ES6 you could do require("marty/es6") or something

@jhollingworth
Copy link
Contributor Author

We're feature complete on this release so I've publish Marty v0.9.0-rc1. I've updated the chat example (including isomorphism) if anyone is interested in what it looks like

@dariocravero
Copy link
Contributor

Brilliant!! :) It's going to be an amazing release!

I was wondering if it should actually be 1.0.0 as it does include a fairly good amount of changes (despite the old mode just being deprecated). What would your plans for a 1.0.0 release be otherwise?

I'm working on a fully class-ified structure right now, will put that up as an example too. Will have a look at the chat example later on today and comment on it.

@jhollingworth
Copy link
Contributor Author

Good point about v1.0. My only concern is React isn't stable yet so we might have to change the APIs (e.g. How we use contexts). Maybe it's worth waiting until React 1.0 comes out?

On 9 Mar 2015, at 1:19 pm, Darío Javier Cravero notifications@github.com wrote:

Brilliant!! :) It's going to be an amazing release!

I was wondering if it should actually be 1.0.0 as it does include a fairly good amount of changes (despite the old mode just being deprecated). What would your plans for a 1.0.0 release be otherwise?

I'm working on a fully class-ified structure right now, will put that up as an example too. Will have a look at the chat example later on today and comment on it.


Reply to this email directly or view it on GitHub.

@dariocravero
Copy link
Contributor

Good stuff :) In all fairness, 0.9.0 add functionality and is still backwards compatible, so it's alright :)

@jhollingworth
Copy link
Contributor Author

Last major blocker (beyond pending tasks) is react-router v0.13 remix-run/react-router#638

@dariocravero
Copy link
Contributor

On that: #186 :). The way I see it, an agnostic framework wouldn't make assumptions as to what routing you want to use. Therefore react-router shouldn't be enforced and should be hidden behind a facade that allows any router to be plugged in. Am I wrong to think that marty might be better off being agnostic to have greater acceptance/usage and granularity? Thoughts?

@jhollingworth
Copy link
Contributor Author

Oh completely. its just that marty-express currently only works with react-router (although plan is to allow any router to integrate). Isomorphisim is one of the major features for v0.9 so want everything working together before I release

@dariocravero
Copy link
Contributor

Class! Good to know it was already on your mind too :) I'm glad to help towards getting the next version on a modular approach if that's desirable

@FoxxMD
Copy link

FoxxMD commented Mar 11, 2015

Along with #153 and #156 I'd like to add that there should be an UPGRADING.md document (or include in one of the docs) in the case that there are any breaking changes between 0.8 and 0.9.

@dariocravero
Copy link
Contributor

I'll be running an upgrade over the next few weeks (probably next week) on
a fairly large 0.8 project to 0.9 with classes. I think I should be able to
create a guide out of that documenting the process and any gotchas in
between. It shouldn't really break anything with your current code though.
On 11 Mar 2015 15:06, "Matt" notifications@github.com wrote:

Along with #153 #153 and #156
#156 I'd like to add that
there should be an UPGRADING.md document (or include in one of the docs)
in the case that there are any breaking changes between 0.8 and 0.9.


Reply to this email directly or view it on GitHub
#137 (comment).

@FoxxMD
Copy link

FoxxMD commented Mar 11, 2015

Thanks @dariocravero that'd be awesome 👍

@jhollingworth
Copy link
Contributor Author

I've just finished a big update to Marty DevTools. Load of bug fixes, immutable.js support, a better view on the data plus the ability to revert actions

@jhollingworth
Copy link
Contributor Author

v0.9 docs are available here http://martyjs.org/v/0.9.0-rc2/

@dariocravero
Copy link
Contributor

@FoxxMD here's a rough guide (by example). Will try to write it up a bit better these days.

@FoxxMD
Copy link

FoxxMD commented Mar 24, 2015

thanks!

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

Successfully merging this pull request may close these issues.

5 participants