Skip to content

Commit

Permalink
Build: Use file-loader to load flag SVG (#26555)
Browse files Browse the repository at this point in the history
Use webpack's `file-loader` to copy flag SVG assets to `public/images` when it encounters code that `require()`s them. This allows us to drop the unconditional copying we previously did through `CopyWebpackPlugin`, and the code that disabled it for the SDK. Henceforth, webpack will be able to determine on its own whether to include flags SVGs when bundling, depending on whether there's a `require()` that calls for them.

Note we're now copying to `public/images/` rather than `public/images/flags/`, since the `file-loader` is generic enough and agnostic of flags to be able to copy any kind of asset whose extension we whitelist -- currently that's only `.svg`, but doesn't mean it's always going to be just flags. 

In order not to break tests (or have them unduly depend on webpack loaders), we're also adding a Jest helper that essentially translates `require( 'path/to/some.svg' )` to `'some.svg'` (see https://jestjs.io/docs/en/webpack#handling-static-assets).
  • Loading branch information
ockham committed Aug 20, 2018
1 parent 62c3d46 commit 73dd705
Show file tree
Hide file tree
Showing 15 changed files with 279 additions and 246 deletions.
3 changes: 1 addition & 2 deletions .gitignore
Expand Up @@ -37,8 +37,7 @@ test-results*.xml
/public/emergent-paywall*.css
/public/sections/*
/public/sections-rtl/*
/public/images/flags/*.svg
/public/images/flags/*.png
/public/images/*.svg
/server/devdocs/search-index.js
/server/devdocs/components-usage-stats.json
/server/devdocs/proptypes-index.json
Expand Down
1 change: 0 additions & 1 deletion bin/sdk-cli.js
Expand Up @@ -29,7 +29,6 @@ const getBaseConfig = ( options = {} ) => {

// these are currently Calypso-specific
const omitPlugins = [
require( path.resolve( __rootDir, 'server/bundler/copy-webpack-plugin' ) ),
webpack.HotModuleReplacementPlugin,
];

Expand Down
7 changes: 5 additions & 2 deletions client/components/phone-input/country-flag.jsx
Expand Up @@ -45,14 +45,17 @@ export default class extends React.Component {

renderFlag = () => {
const style = this.state.ready ? {} : { visibility: 'hidden' };
const { countryCode } = this.props;

if ( this.props.countryCode ) {
if ( countryCode ) {
if ( ! this.state.error ) {
const flagSvg = require( `flag-icon-css/flags/4x3/${ countryCode }.svg` );
return (
<img
alt=""
onLoad={ this.handleImageLoad }
onError={ this.handleImageError }
src={ `/calypso/images/flags/${ this.props.countryCode }.svg` }
src={ flagSvg }
className="phone-input__flag-icon"
style={ style }
/>
Expand Down
Expand Up @@ -20,6 +20,7 @@ class LocationFlag extends Component {
render() {
const { code, className } = this.props;
const { ready } = this.state;
const flagSvg = require( `flag-icon-css/flags/4x3/${ code.toLowerCase() }.svg` );
const style = ready ? {} : { visibility: 'hidden' };
const onLoad = () => this.setState( { ready: true } );
const onError = () => this.setState( { ready: false } );
Expand All @@ -30,7 +31,7 @@ class LocationFlag extends Component {
onError={ onError }
className={ classNames( 'location-flag', className ) }
style={ style }
src={ `/calypso/images/flags/${ code.toLowerCase() }.svg` }
src={ flagSvg }
alt=""
/>
);
Expand Down
12 changes: 6 additions & 6 deletions client/state/stats/lists/test/utils.js
Expand Up @@ -706,7 +706,7 @@ describe( 'utils', () => {
countryCode: 'US',
value: 1,
region: '021',
backgroundImage: '/calypso/images/flags/us.svg',
backgroundImage: 'us.svg',
},
] );
} );
Expand Down Expand Up @@ -750,7 +750,7 @@ describe( 'utils', () => {
countryCode: 'US',
value: 10,
region: '021',
backgroundImage: '/calypso/images/flags/us.svg',
backgroundImage: 'us.svg',
},
] );
} );
Expand Down Expand Up @@ -793,7 +793,7 @@ describe( 'utils', () => {
countryCode: 'US',
value: 100,
region: '021',
backgroundImage: '/calypso/images/flags/us.svg',
backgroundImage: 'us.svg',
},
] );
} );
Expand Down Expand Up @@ -837,7 +837,7 @@ describe( 'utils', () => {
countryCode: 'US',
value: 100,
region: '021',
backgroundImage: '/calypso/images/flags/us.svg',
backgroundImage: 'us.svg',
},
] );
} );
Expand Down Expand Up @@ -880,7 +880,7 @@ describe( 'utils', () => {
countryCode: 'US',
value: 100,
region: '021',
backgroundImage: '/calypso/images/flags/us.svg',
backgroundImage: 'us.svg',
},
] );
} );
Expand Down Expand Up @@ -928,7 +928,7 @@ describe( 'utils', () => {
countryCode: 'US',
value: 100,
region: '021',
backgroundImage: '/calypso/images/flags/us.svg',
backgroundImage: 'us.svg',
},
] );
} );
Expand Down
2 changes: 1 addition & 1 deletion client/state/stats/lists/utils.js
Expand Up @@ -464,7 +464,7 @@ export const normalizers = {

return map( countryData, viewData => {
const country = countryInfo[ viewData.country_code ];
const icon = `/calypso/images/flags/${ viewData.country_code.toLowerCase() }.svg`;
const icon = require( `flag-icon-css/flags/4x3/${ viewData.country_code.toLowerCase() }.svg` );

// ’ in country names causes google's geo viz to break
return {
Expand Down

0 comments on commit 73dd705

Please sign in to comment.