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

fix: urlRegex in custom file handler #292

Closed

Conversation

rodmax
Copy link
Contributor

@rodmax rodmax commented Dec 20, 2017

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

customFileHanlder does not handle any files due to bug in urlRegex regular expression
In my case it broke test where i use file-loader plugin

I guess here is the same issue #291

What is the new behavior?

Fixed regular expression for customFileHandle:

  • removed unneeded escape symbols
  • added function to escape publicPath string. NOTE: this function is not needed when running on linux based OS-es due to os.tmpdir() does not contain special characters, but should help in edge cases(Windows, ???)

short example to explain issue:

    // in my case below url was generated by file-loader
    const url = '/tmp/_karma_webpack_/8f503faad8b590c8a6075b27426dc9c0.json';
    // os.tmpdir() -> "/tmp" in my case(Ubuntu) 
    const originalRe = new RegExp('^/tmp\/_karma_webpack_\/.*/');
    console.log('original urlRegex:', originalRe.test(url));  // false

    const fixedRe = new RegExp('^/tmp/_karma_webpack_/.*');
    console.log('fixed urlRegex:', fixedRe.test(url));  // true

Does this PR introduce a breaking change?

  • Yes
  • No

If this PR contains a breaking change, please describe the following...

  • Impact:
  • Migration path for existing applications:
  • Github Issue(s) this is regarding:

Other information:

@jsf-clabot
Copy link

jsf-clabot commented Dec 20, 2017

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


max.rodionov seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

@rodmax
Copy link
Contributor Author

rodmax commented Dec 20, 2017

it seems i have problems with licence/cla service due to i make commit with my working email([email protected])

So if the work done by me makes sense, I can re-create this pull request with proper commit metadata

What do you think, guys?

@joshwiens
Copy link
Contributor

@rodmax - that's fine though keep in mind that if you are going to do that ( without digging through commit meta ) it will have to be done on a completely new branch

@rodmax
Copy link
Contributor Author

rodmax commented Dec 21, 2017

recreated with proper commit metadata: #293

@rodmax rodmax closed this Dec 21, 2017
@rodmax rodmax deleted the fix-url-regex-in-custom-file-handler branch December 21, 2017 06:18
@michael-ciniawsky michael-ciniawsky changed the title fix: fix "urlRegex" in custom file handler fix: urlRegex in custom file handler Feb 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants