Skip to content

Commit c76a9ae

Browse files
PepsRyuuPepsRyuu
committed
Fixed recursive skipSelf
1 parent 7c95836 commit c76a9ae

File tree

6 files changed

+179
-10
lines changed

6 files changed

+179
-10
lines changed

‎lib/impl/PluginContainer.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ class PluginContainer {
6262
start () {
6363
this.__errorState = false;
6464
this.__errorHandler.reset();
65+
PluginLifecycle.resolveIdSkips.reset();
6566
}
6667

6768
/**

‎lib/impl/PluginContext.js‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,14 +114,17 @@ module.exports = {
114114
* @return {Promise<RollupResolveId>}
115115
*/
116116
async resolve (importee, importer, options = {}) {
117-
let { __plugins } = container;
118-
119117
if (options.skipSelf) {
120-
let index = __plugins.findIndex(p => p.execute === plugin);
121-
__plugins = __plugins.filter((p, i) => i !== index);
118+
PluginLifecycle.resolveIdSkips.add(plugin, importer, importee);
119+
}
120+
121+
try {
122+
return await PluginLifecycle.resolveId(container, importee, importer);
123+
} finally {
124+
if (options.skipSelf) {
125+
PluginLifecycle.resolveIdSkips.remove(plugin, importer, importee);
126+
}
122127
}
123-
124-
return await PluginLifecycle.resolveId(/** @type {PluginContainer} */ ({ __plugins, __config: container.__config }), importee, importer);
125128
},
126129

127130
/**

‎lib/impl/PluginLifecycle.js‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,8 @@ const PluginLifecycle = {
198198
* @return {Promise<RollupResolveIdResult>}
199199
*/
200200
async resolveId (container, id, parentFilePath) {
201-
let hr = await callAsyncFirstHook(container, 'resolveId', [id, parentFilePath]);
201+
let __plugins = container.__plugins.filter(p => !this.resolveIdSkips.contains(p.execute, parentFilePath, id));
202+
let hr = await callAsyncFirstHook(/** @type {PluginContainer} */ ({ __plugins, __config: container.__config }), 'resolveId', [id, parentFilePath]);
202203

203204
if (hr === false || isExternal(container.__config, id)) {
204205
return {
@@ -227,6 +228,32 @@ const PluginLifecycle = {
227228
return hr;
228229
},
229230

231+
resolveIdSkips: {
232+
reset () {
233+
this.storage = new Map();
234+
},
235+
236+
add (plugin, importer, importee) {
237+
let key = importer + '\n' + importee;
238+
let keys = this.storage.get(plugin) || [];
239+
if (keys.indexOf(key) === -1) {
240+
keys.push(key);
241+
}
242+
this.storage.set(plugin, keys);
243+
},
244+
245+
contains (plugin, importer, importee) {
246+
let keys = this.storage.get(plugin) || [];
247+
return keys.indexOf(importer + '\n' + importee) > -1;
248+
},
249+
250+
remove (plugin, importer, importee) {
251+
let keys = this.storage.get(plugin);
252+
keys = keys.filter(k => k !== importer + '\n' + importee);
253+
this.storage.set(plugin, keys);
254+
}
255+
},
256+
230257
/**
231258
* @param {PluginContainer} container
232259
*/

‎lib/impl/PluginMeta.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// @ts-check
22
module.exports = {
3-
rollupVersion: '2.30',
3+
rollupVersion: '2.47',
44
watchMode: true
55
};

‎test/cases/api/context.js‎

Lines changed: 139 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1306,7 +1306,7 @@ describe ('API: Plugin Context', () => {
13061306
input: './src/main.js',
13071307
plugins: [{
13081308
transform () {
1309-
expect(this.meta.rollupVersion).to.equal('2.30');
1309+
expect(this.meta.rollupVersion).to.equal('2.47');
13101310
}
13111311
}]
13121312
});
@@ -1751,6 +1751,144 @@ describe ('API: Plugin Context', () => {
17511751
fs.reset();
17521752
});
17531753

1754+
it ('should allow skipSelf to by applied for multiple plugins recursively at the same time', async function () {
1755+
fs.stub('./src/main.js', () => 'import "jquery"; export default 123');
1756+
fs.stub('./src/lol.js', () => 'export default 456');
1757+
1758+
let passed = false;
1759+
let bundle = await nollup({
1760+
input: './src/main.js',
1761+
plugins: [{
1762+
resolveId (id) {
1763+
if (id === 'jquery') {
1764+
return this.resolve(id, path.resolve(process.cwd(), './src/main.js'), { skipSelf: true }).then(resolved => {
1765+
passed = true;
1766+
expect(resolved.id).to.equal(path.resolve(process.cwd(), './src/lol.js'));
1767+
});
1768+
}
1769+
}
1770+
}, {
1771+
resolveId (id) {
1772+
if (id === 'jquery') {
1773+
return this.resolve('jquery', path.resolve(process.cwd(), './src/main.js'), { skipSelf: true });
1774+
}
1775+
1776+
}
1777+
}, {
1778+
resolveId (id) {
1779+
if (id === 'jquery') {
1780+
return path.resolve(process.cwd(), './src/lol.js');
1781+
}
1782+
}
1783+
}]
1784+
});
1785+
1786+
let { output } = await bundle.generate({ format: 'esm' });
1787+
expect(passed).to.be.true;
1788+
fs.reset();
1789+
});
1790+
1791+
it ('should allow skipSelf to reset after a build failure', async function () {
1792+
fs.stub('./src/main.js', () => 'import "jquery"; export default 123');
1793+
fs.stub('./src/lol.js', () => 'export default 456');
1794+
1795+
let passed = false;
1796+
let phase = 1;
1797+
let bundle = await nollup({
1798+
input: './src/main.js',
1799+
plugins: [{
1800+
resolveId (id) {
1801+
if (id === 'jquery') {
1802+
return this.resolve(id, path.resolve(process.cwd(), './src/main.js'), { skipSelf: true }).then(resolved => {
1803+
passed = true;
1804+
expect(resolved.id).to.equal(path.resolve(process.cwd(), './src/lol.js'));
1805+
});
1806+
}
1807+
}
1808+
}, {
1809+
resolveId (id) {
1810+
if (id === 'jquery') {
1811+
return this.resolve('jquery', path.resolve(process.cwd(), './src/main.js'), { skipSelf: true });
1812+
}
1813+
1814+
}
1815+
}, {
1816+
resolveId (id) {
1817+
if (id === 'jquery') {
1818+
if (phase === 1) {
1819+
throw new Error('Expected failure');
1820+
}
1821+
return path.resolve(process.cwd(), './src/lol.js');
1822+
}
1823+
}
1824+
}]
1825+
});
1826+
1827+
try {
1828+
await bundle.generate({ format: 'esm' });
1829+
} catch (e) {
1830+
expect(e.message).to.equal('Expected failure');
1831+
expect(passed).to.be.false;
1832+
phase++;
1833+
}
1834+
1835+
await bundle.generate({ format: 'esm' });
1836+
expect(passed).to.be.true;
1837+
fs.reset();
1838+
});
1839+
1840+
it ('should adhere to skipSelf only if the module being skipped is resolving the same importer importee pair', async function () {
1841+
fs.stub('./src/main.js', () => 'import "jquery"; export default 123');
1842+
fs.stub('./src/lol.js', () => 'export default 456');
1843+
1844+
let passed = false;
1845+
let bundle = await nollup({
1846+
input: './src/main.js',
1847+
plugins: [{
1848+
resolveId (id, importer) {
1849+
if (id === 'jquery' && importer.indexOf('main.js') > -1) {
1850+
// 1
1851+
return this.resolve(id, path.resolve(process.cwd(), './src/main.js'), { skipSelf: true });
1852+
}
1853+
1854+
if (id === 'jquery' && importer.indexOf('customparent') > -1) {
1855+
// 3
1856+
return this.resolve('jquerynested', 'customparent', { skipSelf: true });
1857+
}
1858+
1859+
if (id === 'jquerynested') {
1860+
throw new Error('should not hit here');
1861+
}
1862+
}
1863+
}, {
1864+
resolveId (id) {
1865+
if (id === 'jquery') {
1866+
// 2
1867+
return this.resolve('jquery', 'customparent', { skipSelf: true });
1868+
}
1869+
1870+
if (id === 'jquerynested') {
1871+
// 4
1872+
return this.resolve('jquerynested', 'customparent', { skipSelf: true }).then(resolved => {
1873+
passed = true;
1874+
expect(resolved.id).to.equal(path.resolve(process.cwd(), './src/lol.js'));
1875+
})
1876+
}
1877+
}
1878+
}, {
1879+
resolveId (id) {
1880+
// 5
1881+
if (id === 'jquerynested') {
1882+
return path.resolve(process.cwd(), './src/lol.js');
1883+
}
1884+
}
1885+
}]
1886+
});
1887+
1888+
await bundle.generate({ format: 'esm' });
1889+
expect(passed).to.be.true;
1890+
fs.reset();
1891+
});
17541892

17551893
it ('should return null if module cannot be resolved by anyone and isn\'t external');
17561894

‎test/cases/api/hooks.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1569,7 +1569,7 @@ describe ('API: Plugin Hooks', () => {
15691569
input: './src/main.js',
15701570
plugins: [{
15711571
options (opts) {
1572-
expect(this.meta.rollupVersion).to.equal('2.30');
1572+
expect(this.meta.rollupVersion).to.equal('2.47');
15731573
passed = true;
15741574
}
15751575
}]

0 commit comments

Comments
 (0)