Skip to content

Commit

Permalink
fix(titleProp): handle the existing title case by using element inste…
Browse files Browse the repository at this point in the history
…ad of value (children) (#315)

Instead of return the children of title from conditional expression, we can return the title element itself e.g. If title is undefined then return the existing title else return:
`<title>{title}</title>`
  • Loading branch information
Sudhir Mitharwal authored and gregberge committed Jun 10, 2019
1 parent afe008d commit 065e7a9
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 26 deletions.
45 changes: 25 additions & 20 deletions packages/babel-plugin-svg-dynamic-title/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,31 +11,36 @@ const plugin = ({ types: t }) => ({
return
}

function getTitleElement(existingTitleChildren = []) {
// create the expression for the title rendering
let expression = t.identifier('title')
// get the existing title string value
const existingTitle = (existingTitleChildren || [])
.map(c => c.value)
.join()
if (existingTitle) {
// if title exists
// render as follows
// {title === undefined ? existingTitle : title}
expression = t.conditionalExpression(
t.binaryExpression('===', expression, t.identifier('undefined')),
t.stringLiteral(existingTitle),
expression,
)
}

// create a title element with the given expression
function createTitle(children = []) {
return t.jsxElement(
t.jsxOpeningElement(t.jsxIdentifier('title'), []),
t.jsxClosingElement(t.jsxIdentifier('title')),
[t.jsxExpressionContainer(expression)],
children,
)
}
function getTitleElement(existingTitleChildren = []) {
const titleExpression = t.identifier('title')
let titleElement = createTitle([
t.jsxExpressionContainer(titleExpression),
])
if (existingTitleChildren && existingTitleChildren.length) {
// if title already exists
// render as follows
const fallbackTitleElement = createTitle(existingTitleChildren)
// {title === undefined ? fallbackTitleElement : titleElement}
const conditionalExpressionForTitle = t.conditionalExpression(
t.binaryExpression(
'===',
titleExpression,
t.identifier('undefined'),
),
fallbackTitleElement,
titleElement,
)
titleElement = t.jsxExpressionContainer(conditionalExpressionForTitle)
}
return titleElement
}

// store the title element
let titleElement
Expand Down
19 changes: 15 additions & 4 deletions packages/babel-plugin-svg-dynamic-title/src/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,23 @@ describe('plugin', () => {
)
})

it('should add title attribute and fallback to existing title', () => {
expect(testPlugin('<svg><title>Hello</title></svg>')).toMatchInlineSnapshot(
`"<svg><title>{title === undefined ? \\"Hello\\" : title}</title></svg>;"`,
it('should add title element and fallback to existing title', () => {
// testing when the existing title contains a simple string
expect(testPlugin(`<svg><title>Hello</title></svg>`)).toMatchInlineSnapshot(
`"<svg>{title === undefined ? <title>Hello</title> : <title>{title}</title>}</svg>;"`,
)
// testing when the existing title contains an JSXExpression
expect(
testPlugin(`<svg><title>{"Hello"}</title></svg>`),
).toMatchInlineSnapshot(
`"<svg>{title === undefined ? <title>{\\"Hello\\"}</title> : <title>{title}</title>}</svg>;"`,
)
})
it('should support empty title', () => {
expect(testPlugin('<svg><title></title></svg>')).toMatchInlineSnapshot(
`"<svg><title>{title}</title></svg>;"`,
)
})

it('should support self closing title', () => {
expect(testPlugin('<svg><title /></svg>')).toMatchInlineSnapshot(
`"<svg><title>{title}</title></svg>;"`,
Expand Down
22 changes: 20 additions & 2 deletions packages/babel-preset/src/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,9 @@ describe('preset', () => {
`)
})
it('should handle titleProp and fallback on existing title', () => {
// testing when existing title has string as chilren
expect(
testPreset('<svg><title>Old</title></svg>', {
testPreset(`<svg><title>Hello</title></svg>`, {
titleProp: true,
state: {
componentName: 'SvgComponent',
Expand All @@ -81,7 +82,24 @@ describe('preset', () => {
const SvgComponent = ({
title
}) => <svg><title>{title === undefined ? \\"Old\\" : title}</title></svg>;
}) => <svg>{title === undefined ? <title>Hello</title> : <title>{title}</title>}</svg>;
export default SvgComponent;"
`)
// testing when existing title has JSXExpression as children
expect(
testPreset(`<svg><title>{"Hello"}</title></svg>`, {
titleProp: true,
state: {
componentName: 'SvgComponent',
},
}),
).toMatchInlineSnapshot(`
"import React from \\"react\\";
const SvgComponent = ({
title
}) => <svg>{title === undefined ? <title>{\\"Hello\\"}</title> : <title>{title}</title>}</svg>;
export default SvgComponent;"
`)
Expand Down

1 comment on commit 065e7a9

@vercel
Copy link

@vercel vercel bot commented on 065e7a9 Jun 10, 2019

Choose a reason for hiding this comment

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

Successfully aliased the URL https://svgr-zgnfrdwcon.now.sh to the following alias.

Please sign in to comment.