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 issue incorrect paths for fonts. Add methods to Use relative paths #179

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions css/style_with_fonts.scss
@@ -0,0 +1,9 @@
$font-path: '../fonts';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should probably be in the fixture/css folder.


@font-face {
font-family: 'Roboto';
src:url('#{$font-path}/Roboto.woff2') format('woff2'),
url('#{$font-path}/Roboto.svg') format('svg');
font-weight: normal;
font-style: normal;
}
643 changes: 643 additions & 0 deletions fixtures/fonts/Roboto.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
12 changes: 12 additions & 0 deletions index.js
Expand Up @@ -75,6 +75,18 @@ const publicApi = {
return this;
},

setFontsPublicPath(publicPath) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add at least a small comment here with some use cases.

webpackConfig.setFontsPublicPath(publicPath);

return this;
},

setImagesPublicPath(publicPath) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing comment for that one too.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, do we really need two methods? is there a use case in which we would only call one of them or call them with different values?

webpackConfig.setImagesPublicPath(publicPath);

return this;
},

/**
* Used as a prefix to the *keys* in manifest.json. Not usually needed.
*
Expand Down
38 changes: 38 additions & 0 deletions lib/WebpackConfig.js
Expand Up @@ -41,6 +41,8 @@ class WebpackConfig {
// Global settings
this.outputPath = null;
this.publicPath = null;
this.imagesPublicPath = null;
this.fontsPublicPath = null;
this.manifestKeyPrefix = null;
this.sharedCommonsEntryName = null;
this.providedVariables = {};
Expand Down Expand Up @@ -133,6 +135,22 @@ class WebpackConfig {
this.publicPath = publicPath;
}

setFontsPublicPath(publicPath) {
// guarantee a single trailing slash
publicPath = publicPath.replace(/\/$/,'');
publicPath = publicPath + '/';

this.fontsPublicPath = publicPath;
}

setImagesPublicPath(publicPath) {
// guarantee a single trailing slash
publicPath = publicPath.replace(/\/$/,'');
publicPath = publicPath + '/';

this.imagesPublicPath = publicPath;
}

setManifestKeyPrefix(manifestKeyPrefix) {
/*
* Normally, we make sure that the manifest keys don't start
Expand Down Expand Up @@ -226,6 +244,26 @@ class WebpackConfig {
return this.runtimeConfig.devServerUrl.replace(/\/$/,'') + this.publicPath;
}

getFontsPublicPath() {
// if we're using webpack-dev-server, use it & add the publicPath
if (this.useDevServer()) {
// avoid 2 middle slashes
return this.runtimeConfig.devServerUrl.replace(/\/$/,'') + this.publicPath;
} else {
return this.fontsPublicPath;
}
}

getImagesPublicPath() {
// if we're using webpack-dev-server, use it & add the publicPath
if (this.useDevServer()) {
// avoid 2 middle slashes
return this.runtimeConfig.devServerUrl.replace(/\/$/,'') + this.publicPath;
} else {
return this.imagesPublicPath;
}
}

addEntry(name, src) {
if (this.entries.has(name)) {
throw new Error(`Duplicate name "${name}" passed to addEntry(): entries must be unique.`);
Expand Down
4 changes: 2 additions & 2 deletions lib/config-generator.js
Expand Up @@ -148,7 +148,7 @@ class ConfigGenerator {
loader: 'file-loader',
options: {
name: filename,
publicPath: this.webpackConfig.getRealPublicPath()
publicPath: this.webpackConfig.getImagesPublicPath()
}
});
}
Expand All @@ -165,7 +165,7 @@ class ConfigGenerator {
loader: 'file-loader',
options: {
name: filename,
publicPath: this.webpackConfig.getRealPublicPath()
publicPath: this.webpackConfig.getFontsPublicPath()
}
});
}
Expand Down
45 changes: 45 additions & 0 deletions test/functional.js
Expand Up @@ -486,6 +486,51 @@ describe('Functional tests using webpack', function() {
});
});

it('setFontsPublicPath() generates relativePath for fonts', (done) => {
const config = createWebpackConfig('www/build', 'dev');
config.setPublicPath('/build');
config.addStyleEntry('css/style', './css/style_with_fonts.scss');
config.setFontsPublicPath('../');
config.enableSassLoader();

testSetup.runWebpack(config, (webpackAssert) => {
webpackAssert.assertManifestPath(
'build/css/style.css',
'/build/css/style.css'
);

webpackAssert.assertOutputFileContains(
'css/style.css',
'src: url(../fonts/Roboto.woff2) format("woff2"), url(/build/images/Roboto.svg) format("svg");'
);

done();
});
});

it('setImagesPublicPath() generates relativePath for images', (done) => {
const config = createWebpackConfig('www/build', 'dev');
config.setPublicPath('/build');
config.addStyleEntry('css/style', './css/style_with_fonts.scss');
config.setFontsPublicPath('../');
config.setImagesPublicPath('../');
config.enableSassLoader();

testSetup.runWebpack(config, (webpackAssert) => {
webpackAssert.assertManifestPath(
'build/css/style.css',
'/build/css/style.css'
);

webpackAssert.assertOutputFileContains(
'css/style.css',
'src: url(../fonts/Roboto.woff2) format("woff2"), url(../images/Roboto.svg) format("svg");'
);

done();
});
});

it('enableSourceMaps() adds to .js, css & scss', (done) => {
const config = createWebpackConfig('www/build', 'dev');
config.setPublicPath('/build');
Expand Down