Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom comment tags with unescaped chars break the regex #13

Open
vzhou842 opened this issue Sep 1, 2018 · 3 comments
Open

Custom comment tags with unescaped chars break the regex #13

vzhou842 opened this issue Sep 1, 2018 · 3 comments
Labels

Comments

@vzhou842
Copy link

vzhou842 commented Sep 1, 2018

Comment tags with parentheses don't work. For example, this loader:

{
  loader: 'webpack-strip-block',
  options: {
    start: 'remove(code)',
    end: 'remove(code)'
  }
}

won't work correctly because it breaks the underlying regex. Any similar unescaped char would probably also break the regex.

If this is intended (allow users to write regex fragments?) it should be documented in the README.

@jballant jballant added the bug label Sep 28, 2018
@jballant
Copy link
Owner

Yes, custom comment tags can be a problem and I probably shouldn't have added support without validating the tags for regex chars. In fact, using a Regex in general is always potentially flawed--though with highly-specific comment tags then it is generally unlkely a problem can occur. But to have a more universal solution, the plugin would need to access and traverse the AST.

I've considered whether custom tags should be allowed at all, for the exact issue that it's not too difficult that people write tags that will break the loader. My options are

  1. Adding validation to check that checks for RegEx chars (e.g. (.\/+? etc.) and throw informative error messages (plus documentation)
  2. Not allow custom tags
  3. Leverage and traverse AST of each module to identify and remove the blocks (a more robust but harder to implement solution that would likely slow down webpack builds slightly).

For the time being, I recommend people only use pretty unique text without regex special chars for the start and end tags. I am leaning towards option 1 as a solution, so at some point--when I have evaluated everything--I will try to work on those changes.

@vzhou842
Copy link
Author

vzhou842 commented Oct 2, 2018

Option 1 seems fine to me. Getting an error message would've been really helpful for me!

@jballant
Copy link
Owner

jballant commented Oct 3, 2018

OK. That's what I'll plan on doing then. Thanks for reporting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants