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

Patternlab should become a concrete class #572

Closed
tburny opened this issue Dec 1, 2016 · 5 comments
Closed

Patternlab should become a concrete class #572

tburny opened this issue Dec 1, 2016 · 5 comments

Comments

@tburny
Copy link

tburny commented Dec 1, 2016

I am using Pattern Lab Node v2.7.0 (dev) on Linux, with Node v4.6.2, using the Grunt Edition.

Currently, the Patternlab "class" is just an object ({})which is extended here and there. Here a "convention over configuration" pattern is applied: Properties of the PL object are added as required.

This approach has the following disadvantages:

  • The overall set of propierties the PL object can have is not defined thoroughly
  • It is difficult to provide some sensible defaults in unit tests. This causes lots of DRY code which could be replaced by something like Patternlab.createEmpty(additionalSettings)
  • It became and becomes more and more a god-object required everywhere

To overcome these problems I propose

  • to create a class with a predefined set of fields, similar to the Pattern class. This also would probably improve overall modularity of the application.
  • Replace some dependencies on the PL class with what is really required for a given method (improves testability and modularity). The goal is to un-god the object a bit
  • See if we can move some methods to the PL object.

The structure would be as follows:
PatternlabEngine has a Patternlab object with the fields engines, package, config, events, graph (#540), patterns, , subtypePatterns, data, listitems, patternSection, patternSectionSubType, header, footer, partials, data.link(for completeness). Also it has at least a method to load+check the configuration from a JSON file. Probably major parts ofpatternlab.js` could eventually be moved into there.
Additionally it might be wise to apply a strategy pattern to how patterns are compiles (compile all, incremental, whatever).

Opinions?

@stale
Copy link

stale bot commented Oct 2, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the needs response label Oct 2, 2017
@stale stale bot closed this as completed Oct 9, 2017
@bmuenzenmeyer bmuenzenmeyer assigned geoffp and unassigned tburny Oct 18, 2017
@bmuenzenmeyer bmuenzenmeyer reopened this Oct 18, 2017
@stale stale bot removed the needs response label Oct 18, 2017
@bmuenzenmeyer
Copy link
Member

@geoffp PING

@bmuenzenmeyer bmuenzenmeyer added this to the 3.0.0 milestone Oct 18, 2017
@geoffp
Copy link
Contributor

geoffp commented Oct 18, 2017

In progress!

@stale
Copy link

stale bot commented Dec 17, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the needs response label Dec 17, 2017
@bmuenzenmeyer
Copy link
Member

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

No branches or pull requests

3 participants