Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

allow config entry to be an array or string #107

Merged
merged 2 commits into from
Oct 2, 2018

Conversation

guybarnard
Copy link
Contributor

Config entry points can be singular or multiple.

In readme, it already that values can an array.

 entry: {
    [name]: [...values]
  },

A real-life example might have both polyfills and main index file:

entry: {
    main: [
      '/src/polyfills.js',
      '/src/index.js'
    ]
  }

This works when adding through the .entry.add().add() interface but does not currently work for merge. The proposed change is one line to automatically make the merge value an array if it is not already.

Copy link
Member

@edmorley edmorley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Thank you for the PR :-)

I don't suppose you could add a test in here?
https://github.com/neutrinojs/webpack-chain/blob/master/test/Config.js

There are some tests for .merge() here that might be useful for inspiration:

test('merge empty', t => {
const resolve = new Resolve();
const obj = {
modules: ['src'],
extensions: ['.js'],
alias: { React: 'src/react' },
};
const instance = resolve.merge(obj);
t.is(instance, resolve);
t.deepEqual(resolve.toConfig(), obj);
});
test('merge with values', t => {
const resolve = new Resolve();
resolve.modules
.add('src')
.end()
.extensions.add('.js')
.end()
.alias.set('React', 'src/react');
resolve.merge({
modules: ['dist'],
extensions: ['.jsx'],
alias: { ReactDOM: 'src/react-dom' },
});
t.deepEqual(resolve.toConfig(), {
modules: ['src', 'dist'],
extensions: ['.js', '.jsx'],
alias: { React: 'src/react', ReactDOM: 'src/react-dom' },
});
});
test('merge with omit', t => {
const resolve = new Resolve();
resolve.modules
.add('src')
.end()
.extensions.add('.js')
.end()
.alias.set('React', 'src/react');
resolve.merge(
{
modules: ['dist'],
extensions: ['.jsx'],
alias: { ReactDOM: 'src/react-dom' },
},
['alias']
);
t.deepEqual(resolve.toConfig(), {
modules: ['src', 'dist'],
extensions: ['.js', '.jsx'],
alias: { React: 'src/react' },
});
});

Many thanks :-)

@edmorley edmorley added the bug label Oct 2, 2018
@edmorley
Copy link
Member

edmorley commented Oct 2, 2018

(Or if you'd prefer I can add the tests - let me know :-))

@guybarnard
Copy link
Contributor Author

Added some tests for merge as indicated. Obviously these test more than just the one line change but agreed they are good to add. Feel free to edit or change the pull request if they don't accommodate the needed tests.

Copy link
Member

@edmorley edmorley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is perfect, thank you!

@edmorley edmorley merged commit 5403abb into neutrinojs:master Oct 2, 2018
@edmorley
Copy link
Member

edmorley commented Oct 3, 2018

This has been released as v4.12.1.

@guybarnard guybarnard deleted the patch-config-entry branch October 3, 2018 02:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

2 participants