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 case where publicPath is a URL #34

Merged
merged 3 commits into from
Jan 9, 2018
Merged

Fix case where publicPath is a URL #34

merged 3 commits into from
Jan 9, 2018

Conversation

jodelamo
Copy link
Contributor

@jodelamo jodelamo commented Dec 22, 2017

Fixes #33

Not sure if others have experienced the exact same issue as me, but this "works on my machine".

To clarify; this part in replaceTag.js

if (prefix) {
  cssFilename = path.join(prefix, cssFilename);
}

Would change a publicPath containing a URL from this:

https://www.example.com/somepath/critical.css

To this (note the missing slash following the protocol):

https:/www.example.com/somepath/critical.css

@ghost
Copy link

ghost commented Dec 29, 2017

Hello.
Yes, the same happens in my project and came up with the same fix too. I almost made a new PR, but luckily I found this one to bump instead.
I've tried this and it fixes the problem. I'm glad to help if there is anything else needed to have this PR merged in master.
Cheers

@numical
Copy link
Owner

numical commented Jan 7, 2018

Thanks!
I'll need to add tests for different types of public path before I can commit.
Will get onto this as soon as I can.
Or if you have time to add to the PR: example test is here.

Giuliano Francisco Vitelli and others added 2 commits January 9, 2018 08:51
@jodelamo
Copy link
Contributor Author

jodelamo commented Jan 9, 2018

@numical Tests added, courtesy of @gvitelli. 🙇

@numical numical merged commit b519716 into numical:master Jan 9, 2018
numical added a commit that referenced this pull request Jan 9, 2018
@numical
Copy link
Owner

numical commented Jan 9, 2018

Thanks all! Version 3.4.6 released with this PR.

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.

Inlined styles link element is not removed from the HTML template
2 participants