DAB0mB / Appfairy

A CLI tool to Migrate a Webflow project into a React app
MIT License
286 stars 39 forks source link

Controllers architecture #3

Closed cray0000 closed 6 years ago

cray0000 commented 6 years ago

Thank you for an 🌟Awesome🌟 project, @DAB0mB !

I want to discuss several things I noticed related to the controller architecture.

The controllers is not part of the auto-magic-generation, but something which the developer fully custom-writes on his own. And currently I can see 2 problems with this:

  1. Controller-components are plugged in into the View-component in a hardcoded way (the name of the Controller has to be magically specific, otherwise nothing will work).

    Possible solution: Precreate controllers/ folder automatically with all the magic Controller files (unless the file already exists) with the basic structure.

  2. sub-Controllers (like ContractFormController).

    The usual usecase for the top-level page Controllers (like IndexController, ThankYouController) is that you'll render them through the router, like react-router.

    But the ContactFormController is going to end up being plugged in into the project by a complete magic. Neither used in the router, neither used anywhere (unless you know that it's being required implicitly in the views/)

    Possible solution: Force developer to use it in the parent Controller -- maybe allow to use the actual ContactFormController somehow instead of the contact-form socket here. Overall creating both: the af-el contact-form AND the af-sock contact-form seemed strange. I think it would be great if af-el can somehow play the af-sock role.

Would be great to know what you think about those items. And thanks for the awesome library once again!

DAB0mB commented 6 years ago

Hey @cray0000. I'm glad to see that people are enjoying my work :-) and thanks for the feedback. I've actually noticed these issues initially, but wasn't sure how I should go with the solution; but it was important for me to release a version asap.

Regards issue 1 - Your solution is pretty nice because it makes things much clearer. I would like to suggest another thing - import the view in the controller module and define the controller explicitly, like so:

import React from 'react'
import MyView from '../views/MyView'

class MyController extends React.Component {
  render() {
    return (
      <MyView />
    )
  }
}

// Notice how we match the view to the controller
export default MyView.Controller = MyController

What do you think of that? It's less strict than the first solution and much more flexible but can potentially cause a poor architecture by the user.

For issue 2 - I don't think that using the socket of an af-el should be mandatory. Sometimes we would like to hide some elements based on the component's state, like a dropdown menu. Also, my thought was enabling the usage of af-el generated components across any React component, it shouldn't be bound to a specific view, so it can be reused, for example - a navbar. I agree that something feels a bit odd with this behavior, but I would like to make some more thinking around it to have a better solution. If you have an alternative to that, do share!

Eytan

DAB0mB commented 6 years ago

I'll think what to do with it. Thank you for brinign it to my attention. Closing as it gets old