formatjs / formatjs-old

The monorepo home to all of the FormatJS related libraries.
https://formatjs.io/
156 stars 53 forks source link

Escaping doesn't seem to fully follow ICU syntax specifications #365

Closed nbouvrette closed 4 years ago

nbouvrette commented 4 years ago

Which package? https://github.com/formatjs/formatjs/tree/master/packages/intl-messageformat

Describe the bug I'm seeing so odd behaviors using single quote escapes that don't seem to match exactly the behavior I found in the ICU documentation.

To Reproduce Steps to reproduce the behavior:

In Chrome console:

Test 1: opening quotes must include closing quotes

console.log((new IntlMessageFormat.IntlMessageFormat("You have '{count'", 'en')).format({count: 1}));

Output (test pass):

You have {count

Test 2: removing the closing quote should return an error

console.log((new IntlMessageFormat.IntlMessageFormat("You have '{count", 'en')).format({count: 1}));

Output (test pass):

Uncaught SyntaxError: Expected "'", "''", ",", or [^'] but end of input found.

Test 3: closing the brace on the argument should not impact the escape behavior

console.log((new IntlMessageFormat.IntlMessageFormat("You have '{count}", 'en')).format({count: 1}));

Output (test fail):

You have '1

Expected result: the variable should not be displayed and the parser should return an error since the quote has not been closed.

Test 4: escape should also apply placeholders (#)

console.log((new IntlMessageFormat.IntlMessageFormat("You {count, plural, one {worked for '#' hour} other {worked for '#' hours}} today.", 'en')).format({count: 1}))

Output (test fail):

You worked for '1' hour today.

Expected result: the variable should not be displayed. The parser should also return an error if we try the same string without closing the quote.

Desktop (please complete the following information):

Additional context

Ref: https://unicode-org.github.io/icu-docs/apidoc/released/icu4j/com/ibm/icu/text/MessageFormat.html

nbouvrette commented 4 years ago

Extra info - just tested using ICU4J and it doesn't seem like the closing quote is necessary - not sure where we can confirm this is part of the specifications on the other hand.

longlho commented 4 years ago

thanks a lot for reporting. We're focusing on the intl-unified-numberformat polyfill so it might take a while to get back to this. PR's definitely always welcome :)

nbouvrette commented 4 years ago

I wish I could do a PR but will barely have time to prepare for the next message format working group demo of my syntax tool - hopefully you can join and give my your thoughts!

pyrocat101 commented 4 years ago

Fixed in https://github.com/formatjs/formatjs/pull/380 (to be released)