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

Auto concat array-like config types #196

Open
searls opened this issue Jan 24, 2014 · 2 comments
Open

Auto concat array-like config types #196

searls opened this issue Jan 24, 2014 · 2 comments

Comments

@searls
Copy link
Member

searls commented Jan 24, 2014

One headache that keeps coming up is high-traffic array-type configuration properties like loadNpmTasks, appendTasks, prependTasks, etc.

If you're writing a plugin, so that you don't stomp on other plugins, you typically have to be sure to use lineman.config.application.loadNpmTasks.concat to edit the loadNpmTasks array, however I just realized through this issue that this also effects the user config for any user who's consuming a plugin.

I didn't run into this myself because I've successfully plugin-ified everything that I use personally.

SO. That sucks.

Because any change will impact multiple deep merge operations I'm going to ask for @jasonkarns thoughts.

@jasonkarns
Copy link
Member

The node extend lib we originally used (+6 months ago) would concat arrays
by default. However, at the time, that lead to a mess. To modify the
loadNpmTasks, you'd have to manually remove everything and re-add, or do
some crazy array manipulation. We moved to a different extend lib
specifically so that it wouldn't merge array properties. Of course, this
was before plugins where the config would be modified multiple times.

With an overwrite model of extend, you can simply declare what the array
should be. Or you can manipulate it using add/remove tasks. Or you can
manipulate it as a proper array with raw JS.

With a concat/merge model of extend, you still have all the same options of
add/remove tasks and plain array manipulation via JS. But you lose the
ability to simply declare the array.

I'm not convinced there's enough reason to move back to the concat/merge
model yet. It doesn't gain any capability but it does lose some.

On Fri, Jan 24, 2014 at 9:27 AM, Justin Searls notifications@github.comwrote:

One headache that keeps coming up is high-traffic array-type configuration
properties like loadNpmTasks, appendTasks, prependTasks, etc.

If you're writing a plugin, so that you don't stomp on other plugins, you
typically have to be sure to use
lineman.config.application.loadNpmTasks.concat to edit the loadNpmTasksarray, however I just realized through this
issue https://github.com/testdouble/lineman-angular-template/issues/31that this also effects the user
config
for any user who's consuming a plugin.

I didn't run into this myself because I've successfully plugin-ified
everything that I use personally.

SO. That sucks.

Because any change will impact multiple deep merge operations I'm going to
ask for @jasonkarns https://github.com/jasonkarns thoughts.


Reply to this email directly or view it on GitHubhttps://github.com//issues/196
.

@searls
Copy link
Member Author

searls commented Jan 24, 2014

Alright, two responses:

  1. I agree that going back to a wholesale merge/concat strategy for arrays would be problematic. I've noodled aroudn a few ideas, such as:
    A. declaring (in the plugin loader) that the task arrays are "special" because they're so high traffic and to do a concat & uniq operation on each of them whenever they're overridden.
    B. publishing special array wrappers/decorations that can hint to lineman the user's intention. So by default, perhaps we use the concat & uniq strategy (for the sake of sparing the most basic users from having to know/think about this), but for everybody else we could publish a lineman.array.redefine([a,b,c]) that would let them totally redefine the array without worrying about concat.
  2. If not changing the merge strategy, what other way can you think of to avoid this problem for end-users?

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