-
-
Notifications
You must be signed in to change notification settings - Fork 27k
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
Named asset import for SVG files #3907
Changes from all commits
45d6324
3a1e13f
3d1de03
f172ee3
8cdc51a
1e2d8a5
d27b488
68439ac
f047ac0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
'use strict'; | ||
|
||
const { extname } = require('path'); | ||
|
||
function namedAssetImportPlugin({ types: t }) { | ||
const visited = new WeakSet(); | ||
|
||
return { | ||
visitor: { | ||
ImportDeclaration(path, { opts: { loaderMap } }) { | ||
const sourcePath = path.node.source.value; | ||
const ext = extname(sourcePath).substr(1); | ||
|
||
if (visited.has(path.node) || sourcePath.indexOf('!') !== -1) { | ||
return; | ||
} | ||
|
||
if (loaderMap[ext]) { | ||
path.replaceWithMultiple( | ||
path.node.specifiers.map(specifier => { | ||
if (t.isImportDefaultSpecifier(specifier)) { | ||
const newDefaultImport = t.importDeclaration( | ||
[ | ||
t.importDefaultSpecifier( | ||
t.identifier(specifier.local.name) | ||
), | ||
], | ||
t.stringLiteral(sourcePath) | ||
); | ||
|
||
visited.add(newDefaultImport); | ||
return newDefaultImport; | ||
} | ||
|
||
const newImport = t.importDeclaration( | ||
[ | ||
t.importSpecifier( | ||
t.identifier(specifier.local.name), | ||
t.identifier(specifier.imported.name) | ||
), | ||
], | ||
t.stringLiteral( | ||
loaderMap[ext][specifier.imported.name] | ||
? loaderMap[ext][specifier.imported.name].replace( | ||
/\[path\]/, | ||
sourcePath | ||
) | ||
: sourcePath | ||
) | ||
); | ||
|
||
visited.add(newImport); | ||
return newImport; | ||
}) | ||
); | ||
} | ||
}, | ||
}, | ||
}; | ||
} | ||
|
||
module.exports = namedAssetImportPlugin; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
{ | ||
"name": "babel-plugin-named-asset-import", | ||
"version": "0.1.0", | ||
"description": "Babel plugin for named asset imports in Create React App", | ||
"repository": "facebookincubator/create-react-app", | ||
"license": "MIT", | ||
"bugs": { | ||
"url": "https://github.com/facebookincubator/create-react-app/issues" | ||
}, | ||
"main": "index.js", | ||
"files": [ | ||
"index.js" | ||
], | ||
"peerDependencies": { | ||
"@babel/core": "7.0.0-beta.38" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -200,6 +200,18 @@ module.exports = { | |
babelrc: false, | ||
// @remove-on-eject-end | ||
presets: [require.resolve('babel-preset-react-app')], | ||
plugins: [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Follow up: this will need an explanatory comment for what it does |
||
[ | ||
require.resolve('babel-plugin-named-asset-import'), | ||
{ | ||
loaderMap: { | ||
svg: { | ||
ReactComponent: 'svgr/webpack![path]', | ||
}, | ||
}, | ||
}, | ||
], | ||
], | ||
compact: true, | ||
highlightCode: true, | ||
}, | ||
|
@@ -308,31 +320,6 @@ module.exports = { | |
), | ||
// Note: this won't work without `new ExtractTextPlugin()` in `plugins`. | ||
}, | ||
// Allows you to use two kinds of imports for SVG: | ||
// import logoUrl from './logo.svg'; gives you the URL. | ||
// import { ReactComponent as Logo } from './logo.svg'; gives you a component. | ||
{ | ||
test: /\.svg$/, | ||
use: [ | ||
{ | ||
loader: require.resolve('babel-loader'), | ||
options: { | ||
// @remove-on-eject-begin | ||
babelrc: false, | ||
// @remove-on-eject-end | ||
presets: [require.resolve('babel-preset-react-app')], | ||
cacheDirectory: true, | ||
}, | ||
}, | ||
require.resolve('svgr/webpack'), | ||
{ | ||
loader: require.resolve('file-loader'), | ||
options: { | ||
name: 'static/media/[name].[hash:8].[ext]', | ||
}, | ||
}, | ||
], | ||
}, | ||
// "file" loader makes sure assets end up in the `build` folder. | ||
// When you `import` an asset, you get its filename. | ||
// This loader doesn't use a "test" so it will catch all modules | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -71,6 +71,22 @@ describe('Integration', () => { | |
); | ||
}); | ||
|
||
it('svg component', async () => { | ||
const doc = await initDOM('svg-component'); | ||
|
||
expect(doc.getElementById('feature-svg-component').textContent).to.equal( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I realize this test is not terribly useful but I'm not sure what I can check for here. I can try and improve it or just remove it entirely. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is for the SVG component not SVG in CSS so I don't think it will have any style? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh okay. Then can’t we test that innerHTML contains some SVG tags? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're not getting a real SVG here because we mock it in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If that works let’s test for that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have no idea if that works (or how to do it) but I'll give it a try. 😀 |
||
'' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still want a better test here |
||
); | ||
}); | ||
|
||
it('svg in css', async () => { | ||
const doc = await initDOM('svg-in-css'); | ||
|
||
expect( | ||
doc.getElementsByTagName('style')[0].textContent.replace(/\s/g, '') | ||
).to.match(/\/static\/media\/logo\..+\.svg/); | ||
}); | ||
|
||
it('unknown ext inclusion', async () => { | ||
const doc = await initDOM('unknown-ext-inclusion'); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
import React from 'react'; | ||
import './assets/svg.css'; | ||
|
||
export default () => <div id="feature-svg-in-css" />; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import React from 'react'; | ||
import ReactDOM from 'react-dom'; | ||
import SvgInCss from './SvgInCss'; | ||
|
||
describe('svg in css', () => { | ||
it('renders without crashing', () => { | ||
const div = document.createElement('div'); | ||
ReactDOM.render(<SvgInCss />, div); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
#feature-svg-in-css { | ||
background-image: url("./logo.svg"); | ||
} |
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.
See comment below