fix(pluginutils)!: don't add cwd to absolute or patterns that start with a glob #517
Conversation
LarsDenBakker
changed the title
|
This definitely seems like a breaking change, even if the previous behavior is considered a bug? Had deja vu reading this, wasn't sure why, then I went back and found rollup/rollup-pluginutils#39 |
|
@lukastaegert I think we'll need some additional historical perspective here again. could you review what @tivac shared and comment? |
|
I guess it can be considered a breaking change as it is always possible that people were relying on e.g. |
shellscape
changed the title
|
@lukastaegert with it being a breaking change, would you approve of this change? |
|
Yes I would. This is a recurring issue, and the solution looks to me like it fits the situations where people had complaints. Of course this means that any plugin updating here will also need a major version if it relies on this. We should keep this in mind when doing major versions of plugins to use the chance to update this dependency as well. |
This includes 2 major version bumps: 2-3: updates minimum rollup version to 1.20 and node version to 8, not a problem 3-4: fixes/changes behavior of createFilter with absolute paths or globs at start, see rollup/plugins#517
This includes 2 major version bumps: 2-3: updates minimum rollup version to 1.20 and node version to 8, not a problem 3-4: fixes/changes behavior of createFilter with absolute paths or globs at start, see rollup/plugins#517
- microbundle is using a relatively older version - 0.29.0: https://github.com/developit/microbundle/blob/555088d17c97860a51ce43808853f4d4f0146a4c/package.json#L111 - meanwhile, 0.30.0 updated the `@rollup/pluginutils` dep: ezolenko/rollup-plugin-typescript2@c6f6e52 - and `@rollup/pluginutils` v4 contains this breaking change: rollup/plugins#517 - with that change, the `filter` now resolves files outside of the cwd - now rpt2 resolves the file correctly and there is no error anymore - that being said, it does produce an empty chunk, but that is correct behavior because Rollup applies treeshaking and the export does nothing - `named` exports are set (https://rollupjs.org/guide/en/#exporting) - and `export * from` actually _doesn't_ export the `default` export - changing this `export { default } from` gets it to output something - note: had to set overrides in root package.json as that's what pnpm requires: https://pnpm.io/package_json#pnpmoverrides
Rollup Plugin Name:
pluginutilsThis PR contains:
Are tests included?
Breaking Changes?
If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.
List any relevant issue numbers: Fixes #490
Description
When a pattern is an absolute path or when it starts with a glob star, the current working directory is not prefixed.
If a pattern is an absolute pattern:
/Users/me/some/path/to/code.js, the pattern becomes/Users/me/some/path/Users/me/some/path/to/code.jswhich is a useless path.If a pattern starts with a glob, like:
**/*the user's intent is to clearly match all parent directories. This becomes:/Users/me/some/path/**/*, and the user (unknowingly) does not include or exclude files outside of the current working directory.I consider this a bugfix because in both cases it goes against the user's intent. However if code is relying on this behavior, it might have an unintended side effect. I think the benefit outweighs this risk.