fix: 🐛 prevent extraction of non-deterministic message ids

✅ Closes: #89

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<string>) {
   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);
     }
   });
 

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<Message>(
-    (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<Message>((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;
+  }, {});
 }

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;
 }

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 = `
+    <script>
+      import { _, defineMessages } from 'svelte-i18n';
+
+      $_({ id: 'foo' + i })
+      $_('bar' + i)
+    </script>`;
+
+    const messages = collectMessages(markup);
+
+    expect(messages).toHaveLength(0);
+  });
 });
 
 describe('messages extraction', () => {
@@ -163,7 +177,7 @@ describe('messages extraction', () => {
       import { _ } from 'svelte-i18n';
     </script>
 
-    <h1>{$_.title('title')}</h1>
+    <h1>{$_('title')}</h1>
     <h2>{$_({ id: 'subtitle'})}</h2>
     `;
 
@@ -176,7 +190,7 @@ describe('messages extraction', () => {
     const markup = `
     <script>import { _ } from 'svelte-i18n';</script>
 
-    <h1>{$_.title('home.page.title')}</h1>
+    <h1>{$_('home.page.title')}</h1>
     <h2>{$_({ id: 'home.page.subtitle'})}</h2>
     <ul>
       <li>{$_('list.0')}</li>
@@ -197,7 +211,7 @@ describe('messages extraction', () => {
     const markup = `
     <script>import { _ } from 'svelte-i18n';</script>
 
-    <h1>{$_.title('home.page.title')}</h1>
+    <h1>{$_('home.page.title')}</h1>
     <h2>{$_({ id: 'home.page.subtitle'})}</h2>
     <ul>
       <li>{$_('list.0')}</li>
@@ -221,7 +235,7 @@ describe('messages extraction', () => {
     const markup = `
     <script>import { _ } from 'svelte-i18n';</script>
 
-    <h1>{$_.title('home.page.title')}</h1>
+    <h1>{$_('home.page.title')}</h1>
     <h2>{$_({ id: 'home.page.subtitle'})}</h2>
     <ul>
       <li>{$_('list.0')}</li>
@@ -256,7 +270,7 @@ describe('messages extraction', () => {
     const markup = `
     <script>import { _ } from 'svelte-i18n';</script>
 
-    <h1>{$_.title('home.page.title')}</h1>
+    <h1>{$_('home.page.title')}</h1>
     <h2>{$_({ id: 'home.page.subtitle'})}</h2>
     `;