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.

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

fix: refactor escapeHtml fn to cast nullish values to empty string #127

Conversation

mostafa-raafat
Copy link
Contributor

No description provided.

@Matwog
Copy link
Contributor

Matwog commented Jan 19, 2022

@mostafa-raafat Could you also please add a fallback value.

  function applyMarkdown(translation = '') {
    // Escape underscores.
    // (Since we use underscores to denote interpolations, we have to
    // exclude them from the markdown notation. Use asterisk (*) instead.)
    let finalTranslation = marked(translation.replace(/_/g, '\\_'))

    // remove single, outer wrapping <p>-tag
    if (isWrappedInPTag(finalTranslation)) {
      // last occurrence of <p> is at the start, first occurrence of </p> is a the very end
      finalTranslation = finalTranslation.substring(
        3,
        finalTranslation.length - 5
      )
    }

    return finalTranslation.replace(/\\_/g, '_')
  }

mostafa-raafat and others added 2 commits January 19, 2022 15:31
add tests to assert the fallback behaviour
@mostafa-raafat
Copy link
Contributor Author

@mostafa-raafat Could you also please add a fallback value.

  function applyMarkdown(translation = '') {
    // Escape underscores.
    // (Since we use underscores to denote interpolations, we have to
    // exclude them from the markdown notation. Use asterisk (*) instead.)
    let finalTranslation = marked(translation.replace(/_/g, '\\_'))

    // remove single, outer wrapping <p>-tag
    if (isWrappedInPTag(finalTranslation)) {
      // last occurrence of <p> is at the start, first occurrence of </p> is a the very end
      finalTranslation = finalTranslation.substring(
        3,
        finalTranslation.length - 5
      )
    }

    return finalTranslation.replace(/\\_/g, '_')
  }

@Matwog Thank you for the suggestion 👍

…lish-values-to-empty-string' of github.com:signavio/i18n into refactor(translate)--refactor-escapeHtml-fn-to-cast-nullish-values-to-empty-string
@Matwog Matwog requested a review from atilafassina January 19, 2022 14:36
@@ -70,7 +70,7 @@ export default (singleton) => {
)
}

function applyMarkdown(translation) {
function applyMarkdown(translation = '') {
Copy link
Contributor

Choose a reason for hiding this comment

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

hint: adds fallback

@@ -93,7 +93,7 @@ export default (singleton) => {
return <span key={key} dangerouslySetInnerHTML={{ __html: html }} />
}

function insertInterpolations(translation, options) {
function insertInterpolations(translation = '', options) {
Copy link
Contributor

Choose a reason for hiding this comment

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

hint: adds fallback

Comment on lines +177 to +193
// Used to map characters to HTML entities.
const htmlEscapes = {
'&': '&amp;',
'<': '&lt;',
'>': '&gt;',
'"': '&quot;',
"'": '&#039;',
}

// Used to match HTML entities and HTML characters.
const reUnescapedHtml = /[&<>"']/g
// Cast (null,undefined,[] and 0 to empty string => '')
const reHasUnescapedHtml = RegExp(reUnescapedHtml.source)

return unsafe && reHasUnescapedHtml.test(unsafe)
? unsafe.replace(reUnescapedHtml, (chr) => htmlEscapes[chr])
: unsafe || ''
Copy link
Contributor

Choose a reason for hiding this comment

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

hint: adds fallback

Copy link

@atilafassina atilafassina left a comment

Choose a reason for hiding this comment

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

🏆

Comment on lines +220 to +229
it('should handle undefined values with markdown', () => {
expect(
i18n(undefined, {
markdown: true,
})
).toStrictEqual([])
})

it('should handle undefined values without markdown', () => {
expect(i18n(undefined, {})).toStrictEqual([])
Copy link
Contributor

Choose a reason for hiding this comment

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

hint: adds tests

@Matwog Matwog merged commit ba3008e into master Jan 19, 2022
@Matwog Matwog deleted the refactor(translate)--refactor-escapeHtml-fn-to-cast-nullish-values-to-empty-string branch January 19, 2022 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants