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

Ability to module-namespace actions #199

Open
markshust opened this issue Jul 12, 2016 · 13 comments
Open

Ability to module-namespace actions #199

markshust opened this issue Jul 12, 2016 · 13 comments

Comments

@markshust
Copy link

Example of actions format is given here:
https://kadirahq.github.io/mantra/#sec-Module-Definition

This lays out the following format for actions:

  actions: {
    myNamespace: {
      doSomething: (context, arg1) => {}
    }
  },

This is ok, however it would be much better to namespace the actions by the module instead. When dealing with a lot of actions within many modules, I'd much rather have this format:

  actions: {
    myModule: {
      myNamespace: {
        doSomething: (context, arg1) => {}
      }
    }
  },

For example, given this layout currently:

/modules/core/actions/
  /index.js
  /foo.js
  /bar.js
/modules/foo/actions/
  /index.js
  /foo.js

Since actions are not namespaced per module, it's very easy to overwrite and cross bounds of other files. Right now, calling actions.foo.setTitle, with the above structure, leads to a lot of problems.

This leads one to currently do the following:

/modules/core/actions/
  /index.js
  /coreFoo.js
  /coreBar.js
/modules/foo/actions/
  /index.js
  /fooFoo.js

This properly resolves the ambiguity problem, because now I can call actions.coreFoo.setTitle, and actions.fooFoo.setTitle, and I know which one I'm calling, however this is not DRY.

It would be much better to be able to have this layout:

/modules/core/actions/
  /index.js
  /foo.js
  /bar.js
/modules/foo/actions/
  /index.js
  /foo.js

and properly module-namespace as follows:

  actions: {
    core: {
      foo: {
        setTitle: (context, arg1) => {}
      }
    }
  },

So, then the action can be called with actions.core.foo.setTitle and actions.foo.foo.setTitle.

markshust added a commit to markshust/mantra-core that referenced this issue Jul 12, 2016
markshust added a commit to markshust/react-simple-di that referenced this issue Jul 12, 2016
@markshust
Copy link
Author

I submitted PR's for both mantra-core and react-simple-di. Everything is backwards compatible with the current format as well, so it will work with non-namespaced actions also.

@fermuch
Copy link

fermuch commented Jul 12, 2016

👍

@nadeemja
Copy link

@sungwoncho Could you implement this in mantra-cli?

Thanks :)

@sungwoncho
Copy link
Contributor

@nadeemja I see that the PRs are still pending. Let's talk about implementing it in CLI after this issue is closed.

@nadeemja
Copy link

@sungwoncho Generally I would agree with you, but can we really expect any more progress on this issue? @arunoda has officially retired, and lost interest in meteor. Probably the same can be said about Kadira as a whole. Perhaps you could take over as a maintainer? Or, @theodorDiaconu?

@macrozone
Copy link

As mantra is not about meteor (it is in fact an architecture pattern for react-apps), i am not sure, if @arunoda has "retired" from this project as well.

I strongly advice to keep mantra living, as it solves real problems with modern react-apps.

I also would like to have module-namespaced actions and I hope we can merge that. I would like to help contributing to mantra-core if possible.

@sungwoncho
Copy link
Contributor

There is a thread open on Mantra talk here. Let's wait for @arunoda to weigh in on the thread to clarify where he stands with Mantra project. Then, if necessary, we as a community can make adjustments to keep the project alive.

@tomitrescak
Copy link

To be hones, I'm out of Meteor world as well, writing pure Node.JS + React + MobX + Apollo apps, but my applications are structured using Mantra specification. Yet I do not use Mantra's Komposer but wrote on super simple one for Mobx. Mantra is really helpful and should be maintained, it is a really rich source of knowledge.

@VikR0001
Copy link

The app I'm developing is in Meteor and Mantra, and I'm very glad to have Mantra handling front-end code. Since Mantra is back-end agnostic, I'm glad to have the freedom to move to a different back-end if I ever wish to do so.

@markshust
Copy link
Author

i'm really not sure why all the pr's for this haven't been merged in. i have this working in a production app, submitted pr's with full tests, and it's fully backwards-compatible.

@VikR0001
Copy link

I'm curious why we haven't yet heard from @arunoda here, and/or on Mantra Talk, about his plans for Mantra. Since Mantra was developed from the ground up to be back-end agnostic, and have no required dependencies on Meteor, one would think he would still be involved in Mantra. If for any reason he is not continuing to develop Mantra, he should make that known so the Mantra community can take over.

@smooJitter
Copy link

@markoshust I'm not sure if this was ever merged but I do appreciate your fix

@markshust
Copy link
Author

markshust commented Jun 8, 2017 via email

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

No branches or pull requests

8 participants