Skip to content
This repository has been archived by the owner on Aug 4, 2021. It is now read-only.

Added dedupe option to prevent bundling the same package multiple times #201

Merged
merged 2 commits into from Apr 6, 2019

Conversation

sormy
Copy link
Contributor

@sormy sormy commented Mar 30, 2019

Added dedupe option to prevent bundling the same package multiple times with unit tests

Very useful feature to make sure that specific dependency will be bundled only once and root node_modules version will be used. When npm link is used with a lot of packages it could cause specific package version to be in each package's node_modules and bundling for this scenario will cause that multiple versions of the same package will be bundled even if it is exactly the same version. Usually this is solved by defining package as peer dependency but this package could be also included in devDependencies for tests and this causes it to appear under node_modules.

@sormy
Copy link
Contributor Author

sormy commented Mar 30, 2019

The test is failing on "allows custom options" for NodeJS v6.x only.

@sormy
Copy link
Contributor Author

sormy commented Mar 31, 2019

I can confirm that regression for this failing test on node 6.x was introduced in commit 1eff8d7, most likely caused by package-lock.json update.

My change is good and don't break any tests, safe to merge.

I can add something like below to skip this test on node v6.x but it is completely unrelated to my change:

	it( 'allows custom options', () => {
		// node v6.x support for this feature was broken in 1eff8d75eaa914ff7ffc1663f9b34826e1c8f71d
		if (/^6./.test(process.versions.node)) {
			return;
		}
                 ...
        }

@sormy
Copy link
Contributor Author

sormy commented Apr 2, 2019

Hi, any update?

Copy link
Member

@lukastaegert lukastaegert left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me! Could you please merge in latest master and add the new option to the index.d.ts typings as well? This should also solve the test issues.

@sormy
Copy link
Contributor Author

sormy commented Apr 5, 2019

done

@lukastaegert lukastaegert merged commit 86905f4 into rollup:master Apr 6, 2019
@philipwalton
Copy link

Shouldn't the dedupe option default to deduping everything, and then you can provide exceptions if you know you want more than one version of a particular package?

As it is now, a user would have to first discover that duplication was occurring and then proactively use this option to fix it. Discovering duplication in a bundle isn't easy, so by not having it be the default, I think this will solve very few people's problems.

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

Successfully merging this pull request may close these issues.

None yet

3 participants