MithrilJS / mithril.js

A JavaScript Framework for Building Brilliant Applications
https://mithril.js.org
MIT License
13.99k stars 926 forks source link

Mithril Realworld Example App #1823

Closed barryels closed 2 years ago

barryels commented 7 years ago

I'm busying setting up a Mithril implementation of the Realworld app.

Would really appreciate feedback / contributions at barryels/realworld-mithril or (more high level) at: https://github.com/gothinkster/realworld/issues/69

tivac commented 7 years ago

This is a neat idea, will take a look this evening and probably send a few PRs.

barryels commented 7 years ago

Brilliant, thanks @tivac :+1:

barryels commented 7 years ago

@tivac, take a look at: https://github.com/gothinkster/react-redux-realworld-example-app and https://github.com/gothinkster/angular2-realworld-example-app as examples of other implementations.

barryels commented 7 years ago

Hi everyone, just an update:


TL;DR

I'm extremely excited about Mithril (have been using it in production for at least a year) and would love for it to become more mainstream, it's one of those libs that allows you to leave work at 17:00 :+1: Thanks to everyone for helping to grow and maintain this project!

tivac commented 7 years ago
  1. It's probably the most "idiomatic mithril" to do ES5 + m(), but use whatever you're comfortable with IMO. We use ES6 + m() and transpile.
  2. I think RouteResolvers + Layout components are rad, so I vote for using 'em
  3. KISS, with comments about potential improvements & further abstractions if necessary. We tend to use a single state object w/ a simple functional API.
  4. ¯\_(ツ)_/¯ if other "RealWorld" projects have 'em it's probably a good idea.
orbitbot commented 7 years ago
  1. One of the first thoughts that I had when learning about this project is that it would leave room for different interpretations, not only in how to organise the views / controllers and such, but also in how to implement the appropriate model. I've avoided looking at some implementations in the interest of maybe giving it a go on my own at some later point. I would 👎 on using JSX if you feel like writing an "idiomatic" app. On the other hand, it might be easier for React people to compare with JSX. Over time it would probably be good to have multiple projects with different techniques, so the important thing would perhaps be to label/rename the repo properly, since that seems to be how RealWorld sorts different implementations.

  2. I'd use Mithril to its full potential as it makes sense to you.

  3. Haven't looked at the implementations, don't understand the question 😉

  4. I'm for BTDD on principle, however don't really do that much in throwaway projects, and the money is in testing the model layer rather than the view layer anyways...

barryels commented 7 years ago

Awesome! Thanks for the feedback @tivac

  1. Will leave it as-is then (ES5 + m()).
  2. Cool, will leave as-is for now, possibly adding a second layout comp to wrap a screen or two at a later stage.
  3. Rad, I'll conjoin and simplify domain & api-adapter.
  4. The other projects don't seem to, but it could be a good opp to showcase mithrils simple testing approach.
StephanHoyer commented 7 years ago

What about SSR? Do you plan to do this? I can assist in this space if you want

barryels commented 7 years ago

Thanks for the feedback @orbitbot :)

  1. Yeah, was initially going to create a mithril-mobx version, but wanted to try an "idiomatic" approach first. Looking at the other projects has caused me to change my approach in some areas, so as to ensure that the Mithril version is as close to an "apples-to-apples" comparison as is possible. The main reason for alignment with the other projects is to help senior devs easily grok the libraries' differences without getting distracted by app-level details. We can rename the repo later on when the need arises :)
  2. hehehe
  3. Yeah, my thinking as well... very little ROI in "visual" UI testing. Will probably set up a suite for the domain object and ignore the UI for now, unless someone else wants to pick that up ;)
barryels commented 7 years ago

@StephanHoyer Oh cool! Hadn't planned on it, but if you're offering... Then, hell yes!

barryels commented 7 years ago

@tivac Re: 3. domain + api-adapter

Do you mind posting a simple example of what your implementation looks like? I've just retired api-adapter... domain now handles all app-level concerns, might rename it to app.js ;)

dmitriz commented 7 years ago

Just my few cents for what it is worth. I have some Angular experience but got curious with Mithril for it's no nonsense approach... So apologies for my ignorance in advance if there is something I miss.

So following that principle, this

var routes = {
    '/': {
        view: function () {
            return m(LayoutDefault, m(ScreenHome));
        }
    },
    '/article/:slug': {
        view: function () {
            return m(LayoutDefault, m(ScreenArticle));
        }
    },
    '/register': {
        view: function () {
            return m(LayoutDefault, m(ScreenUserRegister));
        }
...
}

might be perhaps possible to shorten to

var routes = {
    '/': () => m(LayoutDefault, m(ScreenHome)),
    '/article/:slug': () => m(LayoutDefault, m(ScreenArticle)),
    '/register': () => m(LayoutDefault, m(ScreenUserRegister)),
...
}

or even

var routes = {
    '/': LayoutDefault(ScreenHome),
    '/article/:slug': LayoutDefault(ScreenArticle),
    '/register': LayoutDefault(ScreenUserRegister),
...
}

The last one would be a portable general syntax with no specific library dependency, and that universality may appeal to people coming from other frameworks (it would to me).

You can even go "functionally crazy" and write it like

var routeConfig = {
    '/': ScreenHome,
    '/article/:slug': ScreenArticle,
    '/register': ScreenUserRegister,
...
}

var routes = R.map(LayoutDefault, routeConfig)
// or
var routes = routeConfig.map(LayoutDefault)

From the code organisation perspective, the components directory looks very busy. Any reasons all components, even unrelated ones, must sit next to each other? This "by-feature-vs-by-function" organisation discussion had been quite popular in angular circles for a quite a while, with the outcome in definite favour of the features.

I have found the directory structure really well done in the angular real world example, with no directory having more than 10 or so files.

Perhaps I would move the LayoutDefault to the higher level, due to its hierarchical role.


Another things that might make it easier to play and rearrange folders, could be to use absolute paths instead of

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

Nothing major, but could make life sweeter a bit :)

Or am I missing any downside here?

tivac commented 7 years ago

@dmitriz That's not how RouteResolvers work. They're a specifically-shaped object that mithril treats differently to enable persistent layout components.

dmitriz commented 7 years ago

@tivac Sure.

I am just saying it might be simpler to write the routes that way for the library consumer, so might be worth to support pure functions instead of the more verbose and complex components, where pure view functions is quite often (always in this app) all you need. (The idea is from @JAForbes)

Basically, there are two ways:

The first way would make it more user-friendly in my view.

Does it make sense?

dmitriz commented 7 years ago

Is there a reason for the null value for what will be called as function?

https://github.com/barryels/realworld-mithril/blob/master/src/ui/components/UserSettingsForm.js#L6

Because Null is the Saruman of Static Typing. :)

barryels commented 7 years ago

@dmitriz Thanks for the feedback, quite valuable discussion points.

  1. Router refactor (RouteResolver): I think one of the goals of this project should be to keep things as simple as possible. Whilst I agree with your sentiment completely (this project source is not a 1:1 version of what I'd build for clients). Each person / team will have their own approach to abstractions, and it would probably hinder the understanding of mithril to add too many layers. Your example refactoring exactly illustrate the power that comes from mithril's simplicity :)

  2. File structure Yup, I'm definitely in the structure-by-feature camp (have worked on some massive angular apps, with feature-toggling, etc.), and organising by feature works most of the time, but there will eventually be a feature request which cross-cuts multiple existing features. Once again, this is a subjective decision, which I'm not willing to make if it's going to detract from mithril's implementation details. Probably best to move Screen*.js and Layout*.js into separate dirs, but other than that I'm not sure that the advantage of feature grouping will outweigh the disadvantage of "hiding" components in separate directories... at this point, at least :)

Example

This component: https://github.com/gothinkster/angularjs-realworld-example-app/master/src/js/components/buttons/favorite-btn.component.js

should probably live here: https://github.com/gothinkster/angularjs-realworld-example-app/master/src/js/article

  1. Absolute require's Nah, absolute pathing in cjs is not a good idea, it creates more problems than it solves; most notably lack of consistent cross-IDE support :/
barryels commented 7 years ago

@dmitriz

Is there a reason for the null value for what will be called as function?

Was just defining the shape of the state object, so that it's clear that some function is going to have to update that reference :)

orbitbot commented 7 years ago

Re: absolute paths, methinky something like rollup's root import is what you'd want, if you're aware of it or not ;)

barryels commented 7 years ago

Ah, yes, requires using es6 modules though?

orbitbot commented 7 years ago

Perhaps in this specific case, there are probably alternatives for other environments & toolchains...

dmitriz commented 7 years ago

@barryels

Thanks for the feedback, quite valuable discussion points.

You are welcome!

Router refactor (RouteResolver): I think one of the goals of this project should be to keep things as simple as possible. Whilst I agree with your sentiment completely (this project source is not a 1:1 version of what I'd build for clients). Each person / team will have their own approach to abstractions, and it would probably hinder the understanding of mithril to add too many layers. Your example refactoring exactly illustrate the power that comes from mithril's simplicity :)

I can see your point. I still hope it can be made less verbose when staying simple. As a workaround, perhaps make the router declarations more direct by importing the components rather than their view methods?

Also, does the router API not allow for any tree hierarchy, where I can put the defaultLayout as a top route and so avoid repeating it every single time?

File structure Yup, I'm definitely in the structure-by-feature camp (have worked on some massive angular apps, with feature-toggling, etc.), and organising by feature works most of the time, but there will eventually be a feature request which cross-cuts multiple existing features. Once again, this is a subjective decision, which I'm not willing to make if it's going to detract from mithril's implementation details. Probably best to move Screen.js and Layout.js into separate dirs, but other than that I'm not sure that the advantage of feature grouping will outweigh the disadvantage of "hiding" components in separate directories... at this point, at least :)

I am myself against deep nesting that are often abused with zillions of tiny files doing almost nothing. But your components are already 2 levels deep, so it wouldn't increase any depth with more subdirs inside src. Right now, as an outsider, I have no idea how to differentiate between different components. Perhaps putting those called by the router higher as index files in their same named directories with other dependent ones beside, then have a separate folder shared with the reusable ones that don't belong elsewhere. I just don't find the name components of much help when everything is a component :)

barryels commented 7 years ago

@dmitriz

RouteResolver: Something more like this? i.e. just DRYing the code up a bit?

var routes = {
    '/': setupRoute(ScreenHome),
    '/article/:slug': setupRoute(ScreenArticle),
    '/register': setupRoute(ScreenUserRegister),
    '/login': setupRoute(ScreenUserLogin),
    '/@:username': setupRoute(ScreenUserProfile),
    '/@:username/favorites': setupRoute(ScreenUserFavorites),
    '/settings': setupRoute(ScreenUserSettings),
    '/editor': setupRoute(ScreenEditor),
    '/editor/:slug': setupRoute(ScreenEditor)
};

function setupRoute(screen, layout) {
    layout = layout || LayoutDefault;

    return {
        view: function () {
            return m(layout, m(screen));
        }
    };
}
dmitriz commented 7 years ago

That would look nicer to me!

Or even restrict the router to the intermediate main slot, then you won't need to wrap them at all. Would that make sense?