From 180fb8ab00ec6d9d1b2d626c4596b8ee10ac6aa2 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Mon, 2 Nov 2026 21:11:41 -0600 Subject: [PATCH] fix(row 333 follow-ups R3): a spread before import is a dynamic import (Vikunja #779) The negative lookbehind in DYNAMIC and in the dynamic branch of IMPORT excluded any dot before `import`. That also skipped the last dot of the spread punctuator, so `${({...import('../x.mjs')})}` returned [] for an existing, a missing and an outside target, and `{...import(name)}` in plain code was neither returned nor refused. Both patterns now share one fragment: a word character or $ before `import`, or a single dot (x.import, x?.import), makes it a property call; a dot that follows another dot ends a spread and does not exclude it. `1..import(` is read as an import too, which errs toward refusal; the header names it. Tests: a spread inside a substitution refuses for an existing, a missing and an outside target and for array spread; a spread in plain code is returned, reaches the target checks, and is refused when its argument is not a plain string; x.import and x?.import stay property calls in plain code and in a substitution. --- packages/cli/tests/business-imports.mjs | 26 +++++++--- packages/cli/tests/business-imports.test.mjs | 53 ++++++++++++++++++++ 2 files changed, 71 insertions(+), 8 deletions(-) diff --git a/packages/cli/tests/business-imports.mjs b/packages/cli/tests/business-imports.mjs index 6e9ab274b..191ece798 100644 --- a/packages/cli/tests/business-imports.mjs +++ b/packages/cli/tests/business-imports.mjs @@ -7,12 +7,14 @@ // Vikunja #779: the helper reads imports with a regex, not a parser, so it stays strict about what // it cannot read. Comments are blanked before matching, so an import in a comment is not one, and an // import form inside a string or a regular expression is not one either. A dynamic import written -// with a space, `import ('./x.mjs')`, counts. These throw: a dynamic import whose argument is not a -// plain string, a dynamic import inside a template substitution (${...}), a file with an unterminated -// string, comment, template or regular expression, a target that is missing or not a regular file, -// and a target whose real path leaves the repository. A substitution with no dynamic import is -// blanked with the rest of the template, as is `${import.meta.url}`; the text of a template and a -// substitution that holds a template of its own are read to their ends. +// with a space, `import ('./x.mjs')`, counts, and so does one after a spread, `{...import('./x.mjs')}`; +// a property call such as `x.import(...)` or `x?.import(...)` does not. `1..import(...)`, a property +// call on a number, reads as an import too, which errs toward refusal. These throw: a dynamic import +// whose argument is not a plain string, a dynamic import inside a template substitution (${...}), a +// file with an unterminated string, comment, template or regular expression, a target that is missing +// or not a regular file, and a target whose real path leaves the repository. A substitution with no +// dynamic import is blanked with the rest of the template, as is `${import.meta.url}`; the text of a +// template and a substitution that holds a template of its own are read to their ends. // The scanner knows line and block comments, quoted strings, template literals, regular expression // literals (by the usual guess from the previous token) and a hashbang. The guess is wrong for a // regular expression after `)` or a keyword, as in `return /x/.test(y)`. After `++` or `--` a slash @@ -23,8 +25,16 @@ import {readdirSync,readFileSync,realpathSync,statSync} from 'node:fs'; import {dirname,join,relative,resolve,sep} from 'node:path'; -const IMPORT=/(?:import|export)\s[^'"`;]*?\bfrom\s*['"](\.{1,2}\/[^'"]+)['"]|\bimport\s*['"](\.{1,2}\/[^'"]+)['"]|(?~^'; const inert=(literals,index)=>literals.some(([from,to])=>index>=from&&indexwithBusiness("const v=`${({...import('../../other/src/real.mjs')})}`;\n",{[`${EXT}real.mjs`]:EMPTY})], + ['a missing target',()=>withBusiness("const v=`${({...import('../../other/src/real.mjs')})}`;\n")], + ['a target outside the repository',()=>tree({[`${BIZ}a.mjs`]:"const v=`${({...import('../../../../outside.mjs')})}`;\n",'outside.mjs':EMPTY})], + ['an array spread of an existing target',()=>withBusiness("const v=`${[...import('../../other/src/real.mjs')]}`;\n",{[`${EXT}real.mjs`]:EMPTY})], +]){ + test(`#779 R3 a spread before import inside a template substitution throws: ${what}`,()=>{ + assert.throws(()=>businessImportedFiles(make()),SUBSTITUTION); + }); +} + +for(const [what,source,files] of [ + ['object spread',"const o={...import('../../other/src/real.mjs')};",{[`${EXT}real.mjs`]:EMPTY}], + ['array spread',"const l=[...import('../../other/src/real.mjs')];",{[`${EXT}real.mjs`]:EMPTY}], +]){ + test(`#779 R3 a spread before import in plain code is a dynamic import and is found when its argument is a plain string: ${what}`,()=>{ + assert.deepEqual(businessImportedFiles(withBusiness(`${source}\n`,files)),['packages/other/src/real.mjs']); + }); +} + +test('#779 R3 a spread before import in plain code reaches the target checks: a missing target throws', ()=>{ + const missing=withBusiness("const o={...import('./missing.mjs')};\n"); + assert.throws(()=>businessImportedFiles(missing),/packages\/business\/src\/a\.mjs imports \.\/missing\.mjs, which does not exist/); +}); + +for(const source of ['const o={...import(name)};','const l=[...import(name)];']){ + test(`#779 R3 a spread before import in plain code is refused when its argument is not a plain string: ${source}`,()=>{ + const repo=withBusiness(`${source}\n`); + assert.throws(()=>businessImportedFiles(repo),/business\/src\/a\.mjs has a dynamic import that is not a plain string literal/); + }); +} + +test('#779 R3 a property call of import, x.import or x?.import, in plain code is not an import', ()=>{ + const repo=withBusiness([ + "loader.import('../../other/src/missing1.mjs');", + "loader?.import('../../other/src/missing2.mjs');", + 'loader.import(name);', + 'loader?.import(name);', + "import '../../other/src/real.mjs';", + ].join('\n'),{[`${EXT}real.mjs`]:EMPTY}); + assert.deepEqual(businessImportedFiles(repo),['packages/other/src/real.mjs']); +}); + +test('#779 R3 a property call of import, x.import or x?.import, in a substitution is not an import and is not refused', ()=>{ + const repo=withBusiness([ + "const u=`${loader.import('../../other/src/missing3.mjs')}${loader?.import('../../other/src/missing4.mjs')}`;", + 'const w=`${loader.import(name)}${loader?.import(name)}`;', + "import '../../other/src/real.mjs';", + ].join('\n'),{[`${EXT}real.mjs`]:EMPTY}); + assert.deepEqual(businessImportedFiles(repo),['packages/other/src/real.mjs']); +}); + test('#779 R2 a slash after a postfix ++ or -- is division, not a regular expression', ()=>{ for(const op of ['++','--']){ const repo=withBusiness([