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 template path resolving regexp to support loader query parameters #542

Merged

Conversation

PeachScript
Copy link
Contributor

@PeachScript PeachScript commented Jan 10, 2017

More details in this issue: #541

@jantimon
Copy link
Owner

Could you tell a little bit more about the changed regexp?

@PeachScript
Copy link
Contributor Author

Of course, I have described the problem in the issue #541 .

@jantimon
Copy link
Owner

Can you please also explain the solution?

@PeachScript
Copy link
Contributor Author

PeachScript commented Jan 11, 2017

Before change, the regular expression /([!])([^/\\][^!?]+|[^/\\!?])($|?.+$)/ will match the following substrings in !!html!swig?raw=true!./src/tmpl/index.html:

  1. ! - prefix
  2. swig - filepath
  3. ?raw=true!./src/tmpl/index.html - postfix

And the getFullTemplatePath function will return the error full template path !!html!/Absolute/path/to/context/swig?raw=true!./src/tmpl/index.swig, because of the regular expression ignore the remaining string if it read a ?, and the substring swig be treated as a file path, the real file path be treated as a part of the postfix.

After change, the regular expression /([!])([^/\\][^!?]+|[^/\\!?])($|?[^!?\n]+$)/ (Only need to look at the bold content) will match the following substrings in !!html!swig?raw=true!./src/tmpl/index.html:

  1. ! - prefix
  2. ./src/tmpl/index.html - filepath
  3. `` - postfix

And the getFullTemplatePath function will return the correct full template path !!html!swig?raw=true!/Absolute/path/to/context/src/tmpl/index.swig, because of the new regular expression doesn't match ! and ? after ?, so it will not treat the loader query parameters as the postfix, this is the solution.


The key source code of the getFullTemplatePath function in index.js:

// Resolve template path
return template.replace(
  /([!])([^/\\][^!?]+|[^/\\!?])($|\?[^!?\n]+$)/,
  function (match, prefix, filepath, postfix) {
    return prefix + path.resolve(filepath) + postfix;
  });

@jantimon
Copy link
Owner

That's quite smart - thanks a lot 👍

@jantimon jantimon merged commit 1b4f8ef into jantimon:master Jan 11, 2017
@PeachScript
Copy link
Contributor Author

Glad you like it :D

@PeachScript
Copy link
Contributor Author

By the way, when do you plan to release the new version?

@PeachScript PeachScript deleted the hotfix/full_template_path_regexp branch January 11, 2017 10:58
@jantimon
Copy link
Owner

Released

@lock
Copy link

lock bot commented May 31, 2018

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants