EvanLovely / theme-tools

Flexible, un-opinionated tools for front end development
MIT License
11 stars 5 forks source link

Improve config merge approach #10

Closed EvanLovely closed 7 years ago

EvanLovely commented 7 years ago

The sass plugin default config has this in it:

const defaultConfig = {
  autoPrefixerBrowsers: [
    'last 2 versions',
    'IE >= 10',
  ],
};

And in my project I use this config:

const config = {
  autoPrefixerBrowsers: [
    'last 2 versions',
  ],
};

Since we're using Lodash's merge (via @theme-tools/core) to merge the two together, we basically get the default config above. Ideally, all of autoPrefixerBrowsers would overwritten when in the user config, so the first instinct is to simply use Object.assign, but then we get into issues concerning nested config like sassdoc:

const defaultConfig = {
  src: [
    'scss/**/*.scss',
  ],
  // more options here...
  sassdoc: {
    enabled: false,
    dest: 'dest/sassdoc',
    verbose: false,
    basePath: '',
    exclude: [],
    theme: 'default',
    sort: [
      'file',
      'group',
      'line>',
    ],
  },
};

Ideally, the user could get set config.sassdoc.enabled to true and then have those other defaults set that are inside the sassdoc object.

EvanLovely commented 7 years ago

If we move to Object.assign, this could be a good disclaimer:

Note: The merge of variables in these files is shallow, if you want to override a single item in a list, you will need to re-define all items in that list.

sghoweri commented 7 years ago

Hmmmm. I might take a peek at this once I get back from my run. Feel like I might have worked through something similar when putting together a couple POC build tasks for the new design system revamp...

sghoweri commented 7 years ago

@EvanLovely something like #12? Is that about right?

EvanLovely commented 7 years ago

Perfect! Thanks!