-
Notifications
You must be signed in to change notification settings - Fork 1
CV-112348 Support dynamic public path and RTL fetching #1
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
CV-112348 Support dynamic public path and RTL fetching #1
Conversation
'if (fetchRTL) {', | ||
Template.indent([ | ||
`fullhref = fullhref.replace(/\\.css/i, '.rtl.css');`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace .css
extension with .rtl.css
if fetchRTL
is enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks weird to me, but so do most webpack plugins.
Is it worth adding some sort of manual tests to that page? Probably not...
@@ -5,7 +5,7 @@ | |||
<meta http-equiv="X-UA-Compatible" content="IE=edge"> | |||
<title>mini-css-extract-plugin testcase</title> | |||
<meta name="viewport" content="width=device-width, initial-scale=1"> | |||
<link rel="stylesheet" type="text/css" href="/dist/preloaded1.css" /> | |||
<link rel="stylesheet" type="text/css" href="/dist/preloaded1.client.css" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This gets a mime/type error when testing, but it is the same on master, so I guess that's a potential PR to the main repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it has to do with the webpack-dev-server
settings. Nothing to worry about though.
filename: '[name].client.css', | ||
chunkFilename: '[name].chunk.client.css', | ||
globalRTLFlag: 'rtlLanguageEnabled', | ||
// outputPublicPath: '/bundles/css/', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// outputPublicPath: '/bundles/css/', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only pertains to the testing environment. Decided to keep this comment for testing later if things need to be adjusted or if the fork needs to pull in changes from the master origin.
This PR contains a:
Motivation / Use-Case
This PR implements 2 new options for this plugin.
globalRTLFlag
Adds the ability to check a global
window
flag at run-time to determine if a RTL stylesheet should be fetched.outputPublicPath
Adds the ability to manually set the request's public path, no longer requiring all requests to match
webpack
'soutput.publicPath
setting.