Skip to content

Conversation

@privatenumber
Copy link
Contributor

@privatenumber privatenumber commented Dec 22, 2020

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

Prior to this PR, eslint-webpack-plugin was not stripping Webpack path queries in resource paths and was failing to resolve files with resource queries in its path (eg. "virtual files" created by Webpack loaders). This is a regression introduced in v2.2.0.

For example, mdvue-loader loads Markdown files as Vue components and also renders Vue code-blocks in the markdown file. To accomplish this, it imports the specific code-block as a virtual module via a query path like this:

/project/README.md?vue&type=script&lang=js&mdvue-file=Demo.vue

Given a ESLintPlugin configuration like this, the .vue at the end of the query makes micromatch detect it as a file:

new ESLintPlugin({
	files: '**/*.{vue,js,md}',
	emitWarning: true,
})

Breaking Changes

Additional Info

  • The fix strips resource queries using url.parse.
  • url.parse is actually deprecated, but decided to still use it as new URL might not be available in older versions of Node.js (globalized in v10). new URL also doesn't support relative paths.
  • It might be fine to just do module.resource.split('?')[0] as well but unsure how well that scales. Seems like macOS allows ? in the filename, and that seems to break url.parse too.

@codecov
Copy link

codecov bot commented Dec 22, 2020

Codecov Report

Merging #58 (8ed4f53) into master (2e6c8eb) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #58   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines          239       239           
  Branches        65        65           
=========================================
  Hits           239       239           
Impacted Files Coverage Δ
src/index.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e6c8eb...8ed4f53. Read the comment docs.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

/cc @ricardogobbosouza I think we need to fix and look at new issues

@decademoon
Copy link

This causes the same module file loaded with different resource queries to be linted multiple times. See #70

@privatenumber
Copy link
Contributor Author

@decademoon

I wouldn't say this caused the bug, but rather surfaced it.

Prior to this PR, this plugin was reading the query as if the file extension was in there. That shouldn't be happening.

I think the solution you propose--tracking linted files to catch duplicates--is a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants