Whitelist files that can be embedded through url-loader (facebook#1305)

* Whitelist files that can be embedded through url-loader Change the current catch-all loader to use file-loader instead of url-loader, and exclude common image file extensons. Add another url-loader for images, configured identically to the original catch-all loader. Part of issue facebook#1293. * Update tests to reflect change in loader config * Integration test fix * More CI fixes * Work around broken loader-utils behaviour in integration tests * Revise the documentation around file loading * Documentation tweaks * Docs tweak * Update test now that webpack/loader-utils has been updated * Update react-scripts to depend on [email protected] * Bump deps
sbuzonas · Mar 7, 2017 · b48e3d2 · b48e3d2
1 parent 558bf20
commit b48e3d2
Show file tree

Hide file tree

Showing 7 changed files with 56 additions and 35 deletions.
diff --git a/config/webpack.config.dev.js b/config/webpack.config.dev.js
@@ -134,15 +134,34 @@ module.exports = {
       // When adding a new loader, you must add its `test`
       // as a new entry in the `exclude` list for "url" loader.
 
-      // "url" loader embeds assets smaller than specified size as data URLs to avoid requests.
-      // Otherwise, it acts like the "file" loader.
+      // "file" loader makes sure those assets get served by WebpackDevServer.
+      // When you `import` an asset, you get its (virtual) filename.
+      // In production, they would get copied to the `build` folder.
       {
         exclude: [
           /\.html$/,
           /\.(js|jsx)$/,
           /\.css$/,
           /\.json$/,
-          /\.svg$/
+          /\.bmp$/,
+          /\.gif$/,
+          /\.jpe?g$/,
+          /\.png$/
+        ],
+        loader: 'file-loader',
+        options: {
+          name: 'static/media/[name].[hash:8].[ext]'
+        }
+      },
+      // "url" loader works like "file" loader except that it embeds assets
+      // smaller than specified limit in bytes as data URLs to avoid requests.
+      // A missing `test` is equivalent to a match.
+      {
+        test: [
+          /\.bmp$/,
+          /\.gif$/,
+          /\.jpe?g$/,
+          /\.png$/
         ],
         loader: 'url-loader',
         options: {
@@ -198,14 +217,6 @@ module.exports = {
             }
           }
         ]
-      },
-      // "file" loader for svg
-      {
-        test: /\.svg$/,
-        loader: 'file-loader',
-        options: {
-          name: 'static/media/[name].[hash:8].[ext]'
-        }
       }
       // ** STOP ** Are you adding a new loader?
       // Remember to add the new extension(s) to the "url" loader exclusion list.

diff --git a/config/webpack.config.prod.js b/config/webpack.config.prod.js
@@ -138,15 +138,32 @@ module.exports = {
       // When adding a new loader, you must add its `test`
       // as a new entry in the `exclude` list in the "url" loader.
 
-      // "url" loader embeds assets smaller than specified size as data URLs to avoid requests.
-      // Otherwise, it acts like the "file" loader.
+      // "file" loader makes sure those assets end up in the `build` folder.
+      // When you `import` an asset, you get its filename.
       {
         exclude: [
           /\.html$/,
           /\.(js|jsx)$/,
           /\.css$/,
           /\.json$/,
-          /\.svg$/
+          /\.bmp$/,
+          /\.gif$/,
+          /\.jpe?g$/,
+          /\.png$/
+        ],
+        loader: 'file-loader',
+        options: {
+          name: 'static/media/[name].[hash:8].[ext]'
+        }
+      },
+      // "url" loader works just like "file" loader but it also embeds
+      // assets smaller than specified size as data URLs to avoid requests.
+      {
+        test: [
+          /\.bmp$/,
+          /\.gif$/,
+          /\.jpe?g$/,
+          /\.png$/
         ],
         loader: 'url-loader',
         options: {
@@ -209,14 +226,6 @@ module.exports = {
           ]
         }, extractTextPluginOptions))
         // Note: this won't work without `new ExtractTextPlugin()` in `plugins`.
-      },
-      // "file" loader for svg
-      {
-        test: /\.svg$/,
-        loader: 'file-loader',
-        options: {
-          name: 'static/media/[name].[hash:8].[ext]'
-        }
       }
       // ** STOP ** Are you adding a new loader?
       // Remember to add the new extension(s) to the "url" loader exclusion list.

diff --git a/fixtures/kitchensink/integration/webpack.test.js b/fixtures/kitchensink/integration/webpack.test.js
@@ -28,8 +28,7 @@ describe('Integration', () => {
     it('no ext inclusion', async () => {
       const doc = await initDOM('no-ext-inclusion')
 
-      expect(doc.getElementById('feature-no-ext-inclusion').textContent)
-        .to.equal('This is just a file without an extension.')
+      expect(doc.getElementById('feature-no-ext-inclusion').href).to.match(/\/static\/media\/aFileWithoutExt\.[a-f0-9]{8}\.bin$/)
     })
 
     it('json inclusion', async () => {
@@ -47,7 +46,7 @@ describe('Integration', () => {
     it('unknown ext inclusion', async () => {
       const doc = await initDOM('unknown-ext-inclusion')
 
-      expect(doc.getElementById('feature-unknown-ext-inclusion').textContent).to.equal('Whoooo, spooky!.')
+      expect(doc.getElementById('feature-unknown-ext-inclusion').href).to.match(/\/static\/media\/aFileWithExt\.[a-f0-9]{8}\.unknown$/)
     })
   })
 })
diff --git a/fixtures/kitchensink/src/features/webpack/NoExtInclusion.js b/fixtures/kitchensink/src/features/webpack/NoExtInclusion.js
@@ -15,5 +15,5 @@ const text = aFileWithoutExt.includes('base64')
   : aFileWithoutExt
 
 export default () => (
-  <p id="feature-no-ext-inclusion">{text}.</p>
+  <a id="feature-no-ext-inclusion" href={text}>aFileWithoutExt</a>
 )
diff --git a/fixtures/kitchensink/src/features/webpack/UnknownExtInclusion.js b/fixtures/kitchensink/src/features/webpack/UnknownExtInclusion.js
@@ -15,5 +15,5 @@ const text = aFileWithExtUnknown.includes('base64')
   : aFileWithExtUnknown
 
 export default () => (
-  <p id="feature-unknown-ext-inclusion">{text}.</p>
+  <a id="feature-unknown-ext-inclusion" href={text}>aFileWithExtUnknown</a>
 )
diff --git a/package.json b/package.json
@@ -27,14 +27,14 @@
     "babel-core": "6.23.1",
     "babel-eslint": "7.1.1",
     "babel-jest": "18.0.0",
-    "babel-loader": "6.3.2",
+    "babel-loader": "7.0.0-alpha.3",
     "babel-preset-react-app": "^2.1.1",
     "babel-runtime": "^6.20.0",
     "case-sensitive-paths-webpack-plugin": "1.1.4",
     "chalk": "1.1.3",
     "connect-history-api-fallback": "1.3.0",
     "cross-spawn": "4.0.2",
-    "css-loader": "0.26.1",
+    "css-loader": "0.26.2",
     "detect-port": "1.0.1",
     "dotenv": "2.0.0",
     "eslint": "3.16.1",
@@ -45,16 +45,16 @@
     "eslint-plugin-jsx-a11y": "4.0.0",
     "eslint-plugin-react": "6.4.1",
     "extract-text-webpack-plugin": "2.0.0",
-    "file-loader": "0.10.0",
+    "file-loader": "0.10.1",
     "fs-extra": "0.30.0",
     "html-webpack-plugin": "2.28.0",
     "http-proxy-middleware": "0.17.3",
     "jest": "18.1.0",
     "object-assign": "4.1.1",
-    "postcss-loader": "1.3.1",
+    "postcss-loader": "1.3.3",
     "promise": "7.1.1",
     "react-dev-utils": "^0.5.2",
-    "style-loader": "0.13.1",
+    "style-loader": "0.13.2",
     "url-loader": "0.5.7",
     "webpack": "2.2.1",
     "webpack-dev-server": "2.4.1",

diff --git a/template/README.md b/template/README.md
@@ -23,7 +23,7 @@ You can find the most recent version of this guide [here](https://github.com/fac
 - [Adding a Stylesheet](#adding-a-stylesheet)
 - [Post-Processing CSS](#post-processing-css)
 - [Adding a CSS Preprocessor (Sass, Less etc.)](#adding-a-css-preprocessor-sass-less-etc)
-- [Adding Images and Fonts](#adding-images-and-fonts)
+- [Adding Images, Fonts, and Files](#adding-images-fonts-and-files)
 - [Using the `public` Folder](#using-the-public-folder)
   - [Changing the HTML](#changing-the-html)
   - [Adding Assets Outside of the Module System](#adding-assets-outside-of-the-module-system)
@@ -466,11 +466,13 @@ Then we can change `start` and `build` scripts to include the CSS preprocessor c
 
 Now running `npm start` and `npm run build` also builds Sass files. Note that `node-sass` seems to have an [issue recognizing newly created files on some systems](https://github.com/sass/node-sass/issues/1891) so you might need to restart the watcher when you create a file until it’s resolved.
 
-## Adding Images and Fonts
+## Adding Images, Fonts, and Files
 
 With Webpack, using static assets like images and fonts works similarly to CSS.
 
-You can **`import` an image right in a JavaScript module**. This tells Webpack to include that image in the bundle. Unlike CSS imports, importing an image or a font gives you a string value. This value is the final image path you can reference in your code.
+You can **`import` a file right in a JavaScript module**. This tells Webpack to include that file in the bundle. Unlike CSS imports, importing a file gives you a string value. This value is the final path you can reference in your code, e.g. as the `src` attribute of an image or the `href` of a link to a PDF.
+
+To reduce the number of requests to the server, importing images that are less than 10,000 bytes returns a [data URI](https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Data_URIs) instead of a path. This applies to the following file extensions: bmp, gif, jpg, jpeg, and png. SVG files are excluded due to [#1153](https://github.com/facebookincubator/create-react-app/issues/1153).
 
 Here is an example: