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.

Sign up for GitHub

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

Return useful object instead empty string #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Return useful object instead empty string #3

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,33 @@ function createSymbol(code, id) {
const markup = cheerio.load(code, { xmlMode: true })
const svgMarkup = markup('svg')
const symbolId = svgMarkup.find('title').text() || id
const viewBox = svgMarkup.attr('viewBox')

markup('svg').replaceWith('<symbol/>')
markup('symbol')
.attr('id', symbolId)
.attr('viewBox', svgMarkup.attr('viewBox'))
.attr('viewBox', viewBox)
.append(svgMarkup.children())

return markup.xml('symbol')
return {
content: markup.xml('symbol'),
Copy link
Owner

Choose a reason for hiding this comment

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

Lets go with markup instead of content

Copy link
Author

Choose a reason for hiding this comment

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

For compatibility with webpack options.

id: symbolId,
viewBox,
}
}

function createSprite(symbols) {
return `<svg xmlns="http://www.w3.org/2000/svg">${symbols.join('')}</svg>`
return `<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"><defs>${symbols.join('')}</defs></svg>`
Copy link
Owner

Choose a reason for hiding this comment

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

👍

}

export default function svgSprite(options = {}) {
const { minify = true, outputFolder, ...rest } = options
const {
minify = true,
outputFolder,
publicPath = '',
spriteFilename = 'sprites.svg',
Copy link
Owner

Choose a reason for hiding this comment

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

Lets go with just filename

Copy link
Author

Choose a reason for hiding this comment

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

TLDR: for compatibility with webpack loader config file.

All options names comes from webpack loader. I think about those guys, who switching from webpack to rollup. They can preserve all the options from webpack version. Or (and this my particular case) who support 2 parallel build system when on transition process from one bundler to anoher. Or webpack for dev, rollup for production. Or just for fun )

...rest
} = options

if (!outputFolder) {
throw new Error('"outputFolder" must be set')
Expand Down Expand Up @@ -58,11 +69,12 @@ export default function svgSprite(options = {}) {
}

const filename = path.basename(id, '.svg')
const { content, viewBox, id: symbolId } = createSymbol(code, filename)

convertedSvgs.set(id, createSymbol(code, filename))
convertedSvgs.set(id, content)

return {
code: ''
code: `export default {id: '${symbolId}_usage', viewBox: '${viewBox}', url: '${publicPath}${spriteFilename}#${symbolId}', toString() { return this.url },}`
Copy link
Owner

Choose a reason for hiding this comment

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

I haven't used webpack svg loader, can you explain me, why we need to add _usage to symbol id here and what are url and toString() used for?

Copy link
Author

@warmrobot warmrobot Dec 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Well, _usage comes from here
https://github.com/nightflash/svg-baker/blob/master/packages/svg-baker/lib/symbol.js#L42
Actually, there is -usage, I made a typo. So you can refer to <use> tag. Set fill to another color or something. And id is for symbol itself.

url - is a full path to external svg, including target hash.
like /staticServerPath/img/sprite.svg#logo-usage

toString is a really nice lifehack when you really not need all the properties of this object, but just url

const spriteObj = {
    id: 'aaaId',
  	url: 'someStatic/svg.sprite#twi-usage',
    toString() { return this.url }
}
// I need id!
console.log(spriteObj.id)

// Just give me url, without destructuring or something
console.log(`<svg>${spriteObj}</svg>`)

}
},
async writeBundle() {
Expand All @@ -71,7 +83,7 @@ export default function svgSprite(options = {}) {
const { data } = await svgo.optimize(createSprite(symbols))

await fs.ensureDir(outputFolder)
await fs.writeFile(`${outputFolder}/sprites.svg`, data)
await fs.writeFile(`${outputFolder}/${spriteFilename}`, data)
Copy link
Owner

Choose a reason for hiding this comment

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

Same as above, lets go with just filename

Copy link
Author

Choose a reason for hiding this comment

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

Just for compatibility of configs with webpack.


loadedSvgs.clear()
}
Expand Down