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

update rollup #141

Merged
merged 1 commit into from
Mar 7, 2018
Merged

update rollup #141

merged 1 commit into from
Mar 7, 2018

Conversation

kellyselden
Copy link
Contributor

@kellyselden kellyselden commented Feb 19, 2018

Unblocks #140

Not sure why the test change was necessary, but the new assertion tests the same thing.

@lukastaegert
Copy link
Member

I have opened an issue with rollup to investigate this further as it seems you have stumbled across a bug here: rollup/rollup#1992

@guybedford
Copy link
Contributor

I just cloned this and linked in the current build of Rollup and it is definitely working for me with that test.

@guybedford
Copy link
Contributor

Ah, spoke too soon... looking into it :)

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.

As the latest rollup release should now contain the proper fix for the test regression, I would like you to revert the test changes as IMO, the old version much better showed what the test was about.

test/test.js Outdated
@@ -369,7 +369,7 @@ describe( 'rollup-plugin-node-resolve', function () {
})
]
}).then( bundle => {
assert.deepEqual( bundle.imports.sort(), [] );
assert.equal( bundle.modules.length, 2 );
Copy link
Member

Choose a reason for hiding this comment

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

The latest release of rollup should fix these test regressions.

@kellyselden kellyselden changed the title update rollup and fix test regression update rollup Feb 25, 2018
@kellyselden
Copy link
Contributor Author

I reverted the test changes.

@keithamus keithamus merged commit 36dcabc into rollup:master Mar 7, 2018
@kellyselden kellyselden deleted the rollup branch March 8, 2018 02:39
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants