diff --git a/.cursor/rules/architecture.mdc b/.cursor/rules/architecture.mdc index 7588a85c1..d83fabffa 100644 --- a/.cursor/rules/architecture.mdc +++ b/.cursor/rules/architecture.mdc @@ -12,6 +12,6 @@ All core logic is written once as a `Generator` function (`function *`). Use `yi Never duplicate logic into separate async and sync methods. A single generator serves both paths. -When wrapping an async+sync function pair (e.g. `contains`/`containsSync`, `exists`/`existsSync`, `readFile`/`readFileSync`), use `toLiquidAsync(syncFn, asyncFn?)` which returns a `LiquidAsync` — one function that picks the sync or async implementation based on a leading `sync: boolean` arg. Then `yield` the result inside a generator to let the driver handle it in both modes. +When wrapping an async+sync function pair (e.g. `contains`/`containsSync`, `exists`/`existsSync`, `readFile`/`readFileSync`), use `toLiquidAsync(asyncFn, syncFn?)` which returns a `LiquidAsync` — one function that picks the sync or async implementation based on a leading `sync: boolean` arg. Then `yield` the result inside a generator to let the driver handle it in both modes. See `src/util/async.ts`. diff --git a/src/fs/fs-impl.spec.ts b/src/fs/fs-impl.spec.ts index ceb21d0cd..1258ca78e 100644 --- a/src/fs/fs-impl.spec.ts +++ b/src/fs/fs-impl.spec.ts @@ -1,9 +1,10 @@ import * as fs from './fs-impl' import * as path from 'path' import { mkdtempSync, writeFileSync, symlinkSync, rmSync } from 'fs' -const { join } = path import { tmpdir } from 'os' +const { join } = path + describe('fs-impl', function () { describe('.resolve()', function () { it('should resolve based on root', async function () { @@ -54,7 +55,8 @@ describe('fs-impl', function () { }) }) describe('.contains()', () => { - it('should return false when path is a symlink to outside root', async () => { + const canSymlink = process.platform !== 'win32' + ;(canSymlink ? it : it.skip)('should return false when path is a symlink to outside root', async () => { const root = mkdtempSync(join(tmpdir(), 'liquid-contains-')) const outside = join(tmpdir(), `secret-${Date.now()}.liquid`) writeFileSync(outside, 'x') @@ -69,7 +71,8 @@ describe('fs-impl', function () { }) }) describe('.containsSync()', () => { - it('should return false when path is a symlink to outside root', () => { + const canSymlink = process.platform !== 'win32' + ;(canSymlink ? it : it.skip)('should return false when path is a symlink to outside root', () => { const root = mkdtempSync(join(tmpdir(), 'liquid-contains-')) const outside = join(tmpdir(), `secret-${Date.now()}.liquid`) writeFileSync(outside, 'x') diff --git a/src/fs/loader.spec.ts b/src/fs/loader.spec.ts index 01414d503..81116b2d9 100644 --- a/src/fs/loader.spec.ts +++ b/src/fs/loader.spec.ts @@ -1,4 +1,5 @@ import * as fs from './fs-impl' +import { resolve } from 'path' import { Loader, LookupType } from './loader' import { toValueSync } from '../util/async' @@ -7,7 +8,7 @@ describe('fs/loader', function () { it('should resolve relatively', function () { const loader = new Loader({ relativeReference: true, fs, extname: '' } as any) const candidates = [...loader.candidates('./foo/bar', ['/root', '/root/foo'], '/root/current')] - expect(candidates).toContain('/root/foo/bar') + expect(candidates).toContain(resolve('/root/foo/bar')) }) }) describe('.lookup()', function () { @@ -27,7 +28,7 @@ describe('fs/loader', function () { const mockFs = { ...fs, existsSync: () => true, exists: async () => true, contains: undefined, containsSync: undefined } const loader = new Loader({ relativeReference: true, fs: mockFs, extname: '', partials: ['/root'] } as any) const result = toValueSync(loader.lookup('./foo/bar', LookupType.Partials, true, '/root/current')) - expect(result).toBe('/root/foo/bar') + expect(result).toBe(resolve('/root/foo/bar')) }) }) }) diff --git a/src/fs/loader.ts b/src/fs/loader.ts index 4a42cd650..b0e471e11 100644 --- a/src/fs/loader.ts +++ b/src/fs/loader.ts @@ -32,12 +32,12 @@ export class Loader { } const fs = options.fs this.contains = toLiquidAsync( - fs.containsSync?.bind(fs) || (() => true), - fs.contains?.bind(fs) + fs.contains?.bind(fs) || (async () => true), + fs.containsSync?.bind(fs) || (() => true) ) this.exists = toLiquidAsync( - fs.existsSync.bind(fs), - fs.exists.bind(fs) + fs.exists?.bind(fs) || (async () => false), + fs.existsSync?.bind(fs) ) } diff --git a/src/parser/parser.ts b/src/parser/parser.ts index e55fabe7e..f61f2323e 100644 --- a/src/parser/parser.ts +++ b/src/parser/parser.ts @@ -26,8 +26,8 @@ export class Parser { this.loader = new Loader(this.liquid.options) this.parseLimit = new Limiter('parse length', liquid.options.parseLimit) this.readFile = toLiquidAsync( - this.fs.readFileSync.bind(this.fs), - this.fs.readFile.bind(this.fs) + this.fs.readFile?.bind(this.fs) || (async () => { throw new Error('readFile not implemented') }), + this.fs.readFileSync?.bind(this.fs) ) } public parse (html: string, filepath?: string): Template[] { diff --git a/src/util/async.ts b/src/util/async.ts index d9018af6c..c190cd860 100644 --- a/src/util/async.ts +++ b/src/util/async.ts @@ -4,12 +4,12 @@ export type LiquidAsync any> = (sync: boolean, ...args: Parameters) => ReturnType | Promise> export function toLiquidAsync any> ( - syncFn: F, - asyncFn?: (...args: Parameters) => Promise> + asyncFn: (...args: Parameters) => Promise>, + syncFn?: F ): LiquidAsync { - const asyncImpl = asyncFn || syncFn as any + const syncImpl = syncFn || asyncFn as any return (sync: boolean, ...args: any[]) => { - return sync ? syncFn(...args as Parameters) : asyncImpl(...args as Parameters) + return sync ? syncImpl(...args as Parameters) : asyncFn(...args as Parameters) } } diff --git a/test/e2e/parse-and-render.spec.ts b/test/e2e/parse-and-render.spec.ts index 3a88ad30c..cfca9c2b2 100644 --- a/test/e2e/parse-and-render.spec.ts +++ b/test/e2e/parse-and-render.spec.ts @@ -60,7 +60,8 @@ describe('.parseAndRender()', function () { const html = await engine.parseAndRender(src) expect(html).toBe('true') }) - describe('symlink outside root', function () { + const canSymlink = process.platform !== 'win32' + ;(canSymlink ? describe : describe.skip)('symlink outside root', function () { let root: string, secret: string beforeAll(function () { root = mkdtempSync(join(tmpdir(), 'liquid-e2e-root-')) diff --git a/test/stub/mockfs.ts b/test/stub/mockfs.ts index 1428f14a6..4c9d4cea3 100644 --- a/test/stub/mockfs.ts +++ b/test/stub/mockfs.ts @@ -1,6 +1,6 @@ import { isString, forOwn } from '../../src/util/underscore' import * as fs from '../../src/fs/fs-impl' -import { resolve } from 'path' +import { resolve, sep } from 'path' interface FileDescriptor { mode: string; @@ -33,12 +33,12 @@ export function mock (options: { [path: string]: (string | FileDescriptor) }) { }; (fs as any).contains = async (root: string, file: string) => { root = resolve(root) - if (!root.endsWith('/')) root += '/' + if (!root.endsWith(sep)) root += sep return file.startsWith(root) }; (fs as any).containsSync = (root: string, file: string) => { root = resolve(root) - if (!root.endsWith('/')) root += '/' + if (!root.endsWith(sep)) root += sep return file.startsWith(root) } }