From 9b6adb6538329ecba1e32e2acdca2c4761c1d99c Mon Sep 17 00:00:00 2001 From: Christian Kaisermann Date: Fri, 6 Nov 2020 09:57:22 -0300 Subject: [PATCH] =?UTF-8?q?fix:=20=F0=9F=90=9B=20prevent=20extraction=20of?= =?UTF-8?q?=20non-deterministic=20message=20ids?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ✅ Closes: #89 --- src/cli/extract.ts | 50 ++++++++++++------------ src/cli/includes/getObjFromExpression.ts | 25 ++++++------ src/cli/types/index.ts | 11 ++---- test/cli/extract.test.ts | 44 ++++++++++++++------- 4 files changed, 67 insertions(+), 63 deletions(-) diff --git a/src/cli/extract.ts b/src/cli/extract.ts index f0ebb00..538c1b3 100644 --- a/src/cli/extract.ts +++ b/src/cli/extract.ts @@ -19,20 +19,13 @@ import { Message } from './types'; const LIB_NAME = 'svelte-i18n'; const DEFINE_MESSAGES_METHOD_NAME = 'defineMessages'; const FORMAT_METHOD_NAMES = new Set(['format', '_', 't']); -const IGNORED_UTILITIES = new Set(['number', 'date', 'time']); function isFormatCall(node: Node, imports: Set) { if (node.type !== 'CallExpression') return false; let identifier: Identifier; - if ( - node.callee.type === 'MemberExpression' && - node.callee.property.type === 'Identifier' && - !IGNORED_UTILITIES.has(node.callee.property.name) - ) { - identifier = node.callee.object as Identifier; - } else if (node.callee.type === 'Identifier') { + if (node.callee.type === 'Identifier') { identifier = node.callee; } @@ -150,23 +143,28 @@ export function collectMessages(markup: string): Message[] { ...definitions.map((definition) => getObjFromExpression(definition)), ...calls.map((call) => { const [pathNode, options] = call.arguments; + let messageObj; if (pathNode.type === 'ObjectExpression') { - return getObjFromExpression(pathNode); + // _({ ...opts }) + messageObj = getObjFromExpression(pathNode); + } else { + const node = pathNode as Literal; + const id = node.value as string; + + if (options && options.type === 'ObjectExpression') { + // _(id, { ...opts }) + messageObj = getObjFromExpression(options); + messageObj.id = id; + } else { + // _(id) + messageObj = { id }; + } } - const node = pathNode as Literal; - const id = node.value as string; + if (messageObj?.id == null) return null; - if (options && options.type === 'ObjectExpression') { - const messageObj = getObjFromExpression(options); - - messageObj.meta.id = id; - - return messageObj; - } - - return { node, meta: { id } }; + return messageObj; }), ].filter(Boolean); } @@ -175,28 +173,28 @@ export function extractMessages( markup: string, { accumulator = {}, shallow = false, overwrite = false } = {} as any, ) { - collectMessages(markup).forEach((message) => { - let defaultValue = message.meta.default; + collectMessages(markup).forEach((messageObj) => { + let defaultValue = messageObj.default; if (typeof defaultValue === 'undefined') { defaultValue = ''; } if (shallow) { - if (overwrite === false && message.meta.id in accumulator) { + if (overwrite === false && messageObj.id in accumulator) { return; } - accumulator[message.meta.id] = defaultValue; + accumulator[messageObj.id] = defaultValue; } else { if ( overwrite === false && - typeof dlv(accumulator, message.meta.id) !== 'undefined' + typeof dlv(accumulator, messageObj.id) !== 'undefined' ) { return; } - deepSet(accumulator, message.meta.id, defaultValue); + deepSet(accumulator, messageObj.id, defaultValue); } }); diff --git a/src/cli/includes/getObjFromExpression.ts b/src/cli/includes/getObjFromExpression.ts index ed07628..ed5a40d 100644 --- a/src/cli/includes/getObjFromExpression.ts +++ b/src/cli/includes/getObjFromExpression.ts @@ -3,20 +3,17 @@ import { ObjectExpression, Property, Identifier } from 'estree'; import { Message } from '../types'; export function getObjFromExpression(exprNode: ObjectExpression) { - return exprNode.properties.reduce( - (acc, prop: Property) => { - // we only want primitives - if ( - prop.value.type === 'Literal' && - prop.value.value !== Object(prop.value.value) - ) { - const key = (prop.key as Identifier).name as string; + return exprNode.properties.reduce((acc, prop: Property) => { + // we only want primitives + if ( + prop.value.type === 'Literal' && + prop.value.value !== Object(prop.value.value) + ) { + const key = (prop.key as Identifier).name as string; - acc.meta[key] = prop.value.value; - } + acc[key] = prop.value.value; + } - return acc; - }, - { node: exprNode, meta: {} }, - ); + return acc; + }, {}); } diff --git a/src/cli/types/index.ts b/src/cli/types/index.ts index c0b25a0..4f1c93b 100644 --- a/src/cli/types/index.ts +++ b/src/cli/types/index.ts @@ -1,10 +1,5 @@ -import { Node } from 'estree'; - export interface Message { - node: Node; - meta: { - id?: string; - default?: string; - [key: string]: any; - }; + id?: string; + default?: string; + [key: string]: any; } diff --git a/test/cli/extract.test.ts b/test/cli/extract.test.ts index 6968293..a2cc994 100644 --- a/test/cli/extract.test.ts +++ b/test/cli/extract.test.ts @@ -1,5 +1,3 @@ -// TODO: better tests. these are way too generic. - import { parse } from 'svelte/compiler'; import { @@ -124,7 +122,7 @@ describe('collecting messages', () => { import { _, defineMessages } from 'svelte-i18n'; console.log($_({ id: 'foo' })) - console.log($_.title({ id: 'page.title' })) + console.log($_({ id: 'page.title' })) const messages = defineMessages({ enabled: { id: 'enabled' , default: 'Enabled' }, @@ -141,20 +139,36 @@ describe('collecting messages', () => { expect(messages).toHaveLength(7); expect(messages).toEqual( expect.arrayContaining([ - expect.objectContaining({ meta: { id: 'foo' } }), - expect.objectContaining({ meta: { id: 'msg_1' } }), - expect.objectContaining({ meta: { id: 'msg_2' } }), - expect.objectContaining({ meta: { id: 'msg_3', default: 'Message' } }), - expect.objectContaining({ meta: { id: 'page.title' } }), + expect.objectContaining({ id: 'foo' }), + expect.objectContaining({ id: 'msg_1' }), + expect.objectContaining({ id: 'msg_2' }), + expect.objectContaining({ id: 'msg_3', default: 'Message' }), + expect.objectContaining({ id: 'page.title' }), expect.objectContaining({ - meta: { id: 'disabled', default: 'Disabled' }, + id: 'disabled', + default: 'Disabled', }), expect.objectContaining({ - meta: { id: 'enabled', default: 'Enabled' }, + id: 'enabled', + default: 'Enabled', }), ]), ); }); + + it('ignores non-static message ids', () => { + const markup = ` + `; + + const messages = collectMessages(markup); + + expect(messages).toHaveLength(0); + }); }); describe('messages extraction', () => { @@ -163,7 +177,7 @@ describe('messages extraction', () => { import { _ } from 'svelte-i18n'; -

{$_.title('title')}

+

{$_('title')}

{$_({ id: 'subtitle'})}

`; @@ -176,7 +190,7 @@ describe('messages extraction', () => { const markup = ` -

{$_.title('home.page.title')}

+

{$_('home.page.title')}

{$_({ id: 'home.page.subtitle'})}

  • {$_('list.0')}
    • @@ -197,7 +211,7 @@ describe('messages extraction', () => { const markup = ` -

    {$_.title('home.page.title')}

    +

    {$_('home.page.title')}

    {$_({ id: 'home.page.subtitle'})}

    • {$_('list.0')}
      • @@ -221,7 +235,7 @@ describe('messages extraction', () => { const markup = ` -

      {$_.title('home.page.title')}

      +

      {$_('home.page.title')}

      {$_({ id: 'home.page.subtitle'})}

      • {$_('list.0')}
        • @@ -256,7 +270,7 @@ describe('messages extraction', () => { const markup = ` -

        {$_.title('home.page.title')}

        +

        {$_('home.page.title')}

        {$_({ id: 'home.page.subtitle'})}

        `;