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

Framework: Consider state restructure to modularize by functionality #3012

Closed
aduth opened this issue Oct 12, 2017 · 8 comments
Closed

Framework: Consider state restructure to modularize by functionality #3012

aduth opened this issue Oct 12, 2017 · 8 comments
Assignees
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Question Questions about the design or development of the editor.

Comments

@aduth
Copy link
Member

aduth commented Oct 12, 2017

As the requirements of the editor application have grown, our simplified state file structuring has become increasingly disorganized. We had initially chosen to group actions, effects, reducers, and selectors for all features. While this helped avoid a sprawl of files which can proliferate in Redux-based state folders, it did not help provide a clear picture for how data is structured or how changes flow through the store.

I believe we optimized on the wrong simplification: on concepts of reducers, actions, and selectors, rather than the functionality that a global state store serves to support.

Enter the "Ducks" pattern. In the Ducks approach, reducers and actions are colocated based on functionality. Where currently all reducers are defined in a single reducer.js file and actions in actions.js, in a Ducks approach each distinct feature would define both the reducer and actions in a standalone file.

Proposed Folder Structure

Before:

/actions.js
/reducer.js
/selectors.js
/effects.js

After:

/blocks.js
/selection.js
/notices.js

...where an individual feature file would contain:

// Reducer

export default function reducer( /* ... */ ) {
	// ...
}

// Action Creators

export function createNotice( /* ... */ ) {
	// ...
}

// Selectors

export function getNotices( /* ... */ ) {
	// ...
}

// Effects

export const effects = {
	// ...	
};

This provides a manageable pattern to scale, isolating behaviors of individual features, while retaining a minimal set of files. It lends clarity in that a developer who is interested in the state capabilities of notices needs only review a single file.

Proposed "Modules"

  • Editor (Initialization, UI?)
  • Post
  • Preferences (User?)
  • Blocks
  • Selection
  • Inserter
  • Typing
  • Notices

As a testiment to the benefit of modularizing, per #2761, isolating post logic would simplify an eventual split between a base editor and post-editing-specific behaviors.

Next Steps

I would suggest as part of this transition that we also create a new state directory within editor. Based on the anticipated number of changed files, it may be wise to tackle this as a standalone first step, before moving forward with splitting actions, effects, reducer, and selectors by feature.

Thoughts?

@aduth aduth added Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Question Questions about the design or development of the editor. labels Oct 12, 2017
@aduth aduth self-assigned this Oct 12, 2017
@gziolo
Copy link
Member

gziolo commented Oct 13, 2017

Thanks for opening this discussion. It aligns with what I did when refactoring coediting PR, see: https://github.com/WordPress/gutenberg/pull/1877/files#diff-7aec9bf53d6bd11a374bc5f3b8a44146R1. I went one step further and put reducers and actions in their own file, but I'm happy to merge them together. It should be fine to have them in one file as it will make scanning for action types easier.

In the coediting PR we introduce also middleware, do you think it would be okey to put it in the same file as reducers and actions?

@aduth
Copy link
Member Author

aduth commented Oct 13, 2017

In the coediting PR we introduce also middleware, do you think it would be okey to put it in the same file as reducers and actions?

Both middlewares and effects are a little less obvious, since as side effects, they don't always fit so well in subject-based isolation.

I think the subject files can support them, ideally with consistent named exports (effects, middleware). effects.js has become quite disorganized, so it'd be good for us to at least give it a try to fit within this scope. Otherwise, we could consider a single middleware.js file or middlewares folder of middleware-specific functionality files.

@gziolo
Copy link
Member

gziolo commented Oct 13, 2017

I think the subject files can support them, ideally with consistent named exports (effects, middleware)

Let's start with that, I will update my PR 👍

@jeffpaul
Copy link
Member

This ticket was mentioned in Slack in #core-editor by jeffpaul. View the logs.

@jeffpaul
Copy link
Member

Discussion in today's Gutenberg bug scrub was to close this as it could very well become less relevant as we start to leverage the data module more heavily.

@noisysocks
Copy link
Member

As the requirements of the editor application have grown, our simplified state file structuring has become increasingly disorganized.

This statement seems just as true today as it did when the issue was created—should we re-open? editor/store/selectors.js is well past 1500 lines now 😄

cc. @aduth @gziolo

@aduth
Copy link
Member Author

aduth commented Apr 19, 2018

I'm not overly concerned with the state of state as I once was. Yes, the files are large, but we do have a new pattern of organization which didn't exist at the time of the original issue, which is the modularization of data. I don't know that we need to introduce any new formal patterns for organizing editor itself, but we could perhaps fragment out some data which may not be entirely relevant. One I've had my eye on is the notices state. Viewport is an example of modularized, though I'm not entirely content with the approach there, at least in how it interoperates with components.

@gziolo
Copy link
Member

gziolo commented Apr 23, 2018

Yes, I agree with what @aduth said. We should double check if we can extract some state parts into their own modules first. It seems like editor is the only module that suffers from this issue at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Question Questions about the design or development of the editor.
Projects
None yet
Development

No branches or pull requests

4 participants