Strip default inclusion pattern doesn't include unresolved relative modules

[yannayl]

• Rollup Plugin Name: strip
• Rollup Plugin Version: 1.3.1
• Rollup Version: 1.27.14
• Operating System (or Browser): Ubuntu 18.04
• Node Version: v13.5.0

How Do We Reproduce?

Clone the reproduction repository and follow the instructions in the README file.

Expected Behavior

Both files - resolved and unresolved should be identical and not include any console.log.

Actual Behavior

Files are different, the unresolved contains:

(function () {
	'use strict';

	function foo() { console.log('foo'); }

	foo();
	bar();

}());

[shellscape]

I believe in this scenario you have to place alias before the strip plugin.

It's difficult to parse what you're expecting here based on the description. Perhaps a repo with a reproduction, or using repl.it would be a better demonstration of the problem.

[yannayl]

Changing the order doesn't matter. The issue has to do with the default include minimatch expression **/*.js failing to match ./external/foo.js id for some reason. (Can it be a bug in the createFilter utility?)

I created a minimal reproduction repository

[shellscape]

We'll need to narrow down the "for some reason" bit before we can merge a code change. That's a good practice to get into. Thanks for the reproduction, we'll have a look.

[shellscape]

OK the crux of the problem is that **/*.js as a filter will have a resolution of the current working directory. In my clone of your repro, that ends up looking like this:

/Users/code/forks/rollup_strip_issue_128_repro/**/*.js

Now here's where it gets squirrely - Rollup resolves ./external/foo.js just fine at some point in the process, but in the strip plugin's transform() method, the path hasn't been fully resolved, and the partial path from the alias is all that transform() in strip sees.

The second half of this issue is that micromatch itself doesn't match relative paths:

const micromatch = require("micromatch")

const m = micromatch.matcher('**/*.js');

m('./external/foo.js'); // false
m('/external/foo.js'); // true

I couldn't find anything in micromatch's repo about relative paths. That makes me think that we need @lukastaegert 's take on this:

• Should Rollup core be providing fully resolved paths to the plugins in transform?
• (or) Should createFilter try and resolve a path that begins with a relative ./?
• (or) Should createFilter try and resolve all paths/ids?

I had considered setting options.resolve to false for the strip plugin, but this will likely effect other plugins that use transform as well.

[shellscape]

It also appears that createFilter expects the paths to be fully resolved, based on this test:

plugins/packages/pluginutils/test/createFilter.ts

Lines 87 to 94 in 6284ebe

	test('ignores a falsy resolve value', (t) => {
	const filter = createFilter(['y*'], ['yx'], { resolve: null });
	t.falsy(filter(resolve('x')));
	t.truthy(filter(resolve('ys')));
	t.falsy(filter(resolve('yx')));
	t.falsy(filter(`${resolve('C')}/ys`));
	t.falsy(filter(resolve('C/d/ys')));
	t.falsy(filter('ys'));

[shellscape]

@yannayl if you have the chance, please try your reproduction again with a fresh dependency tree. We made some updates to pluginutils that I think might affect your scenario.

[yannayl]

@shellscape I tried to update the all dependencies, it only updated rollup to v1.29.0. The problem remains.
Committed to the repro repository

[shellscape]

@yannayl you need to update @rollup/pluginutils, not rollup :)

[yannayl]

I am not very fluent in JS development, so I might be missing something, but the repro does not directly depend on @rollup/pluginutils. I tried to add it anyway, e.g. npm install @rollup/pluginutils but it didn't make any difference. Is there a way to force @rollup/plugin-{alias,strip} to use a specific version of @rollup/pluginutils?

I also tried to change the package-lock.json file and force the use of @rollup/pluginutils and remove every instance of rollup-pluginutils but it didn't help (also replaced require('rollup-pluginutils') with require('@rollup/pluginutils') everywhere.

[shellscape]

Your reproduction depends on @rollup/plugin-strip, which in turn depends on @rollup/pluginutils to create the filters that are giving you trouble. You need to make sure your lockfile (either NPM or Yarn) has the latest versions of @rollup/pluginutils, as well as @rollup/plugin-strip.

I was mistaken. It still relies on the deprecated pluginutils https://github.com/rollup/plugins/blob/master/packages/strip/package.json#L42

We'll get that plugin updated and will ping when that's done.

[shellscape]

Hey folks, this one has gone quite stale. We're going to close for housekeeping but we'd happily review any PRs that pop up to resolve this issue.