fix(url): Tree-shake emitting files

[artemjackson]

Rollup Plugin Name: url

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes (bugfixes and features will not be merged without tests)
• no

Breaking Changes?

• yes (breaking changes will not be merged unless absolutely necessary)
• no

Description

Currently @rollup/plugin-url does not respect Rollup's Tree-shaking.
So all imported files are copied to destination folder even if a parent module has been actually tree-shaked by Rollup.

Simplified example:

// foo.js
import png from "./image.png"

export const foo = 42
export const bar = png

// index.js
import { foo } from "./foo.js"

console.log(foo)

This is bundled to:

dist/
   index.js
   image.png

Despite of foo.js was actually tree-shaked:

// index.js
const foo = 42;

console.log(foo);

I'm not sure what the PR is a feature or a bugfix, so some help needed to classify it

[shellscape]

Thanks for opening this PR. I'd definitely call this a bugfix. We'll definitely need some tests around this to verify it's working correctly and to prevent regressions, before we can merge.

[artemjackson]

I've just noticed that sometimes people just need to import some stuff without actually referencing it in the code, example.

And this PR misses such files cuz it relies on Asset.modules which are populated only by actually used (referenced in code) modules.

From that It seems to me that not file.id should be checked in Asset.modules but file importer.id.
In other words assets should be emitted only if parent module is included in Rollup bundle.

So DO NOT MERGE this version :)

I'm going to fix it and provide tests

[artemjackson]

Well, I have added test and fixed prettier issues.

Regarding of this comment: I didn't find a way to "teach" Rollup that such import is intentional and should not be tree-shaked. Btw, I had an idea to check all of the imported file parent modules for not being tree-shaked (i.e. to belong to bundle Asset.modules) but it seems like a bundler job (e.g. to mark modules as tree-shaked somewhere in this.getModuleInfo).

So @shellscape do you have any vision on this?

[lukastaegert]

rollup/rollup#3663 should give you the solution you are looking for.

[shellscape]

@artemjackson please take a look at the commit lukas referenced and let us know if that resolves the issues you mentioned. It looks like this is ready to go now.

[shellscape]

Heads up: Once this hits 60 days without activity the PR will be closed. Please take a look before we reach that point!