From da5345f85ea214b51ce7c86a45932db1d8d840d7 Mon Sep 17 00:00:00 2001 From: Yuki Hattori Date: Tue, 12 Apr 2022 22:54:16 +0900 Subject: [PATCH 1/4] Prevent recursive mkdir if tried to save the file into the root --- src/file.ts | 9 ++++++--- test/__mocks__/mkdirp.ts | 3 --- test/marp-cli.ts | 1 - 3 files changed, 6 insertions(+), 7 deletions(-) delete mode 100644 test/__mocks__/mkdirp.ts diff --git a/src/file.ts b/src/file.ts index 743492d0..8e450b21 100644 --- a/src/file.ts +++ b/src/file.ts @@ -147,9 +147,12 @@ export class File { } private async saveToFile(savePath: string = this.path) { - await fs.promises.mkdir(path.dirname(path.resolve(savePath)), { - recursive: true, - }) + const directory = path.dirname(path.resolve(savePath)) + + if (path.dirname(directory) !== directory) { + await fs.promises.mkdir(directory, { recursive: true }) + } + await fs.promises.writeFile(savePath, this.buffer!) // eslint-disable-line @typescript-eslint/no-non-null-assertion } diff --git a/test/__mocks__/mkdirp.ts b/test/__mocks__/mkdirp.ts deleted file mode 100644 index 425334df..00000000 --- a/test/__mocks__/mkdirp.ts +++ /dev/null @@ -1,3 +0,0 @@ -const mkdirp = jest.fn().mockImplementation((path) => Promise.resolve(path)) - -export default mkdirp diff --git a/test/marp-cli.ts b/test/marp-cli.ts index 7fa30514..0805f773 100644 --- a/test/marp-cli.ts +++ b/test/marp-cli.ts @@ -38,7 +38,6 @@ const runForObservation = async (argv: string[]) => { } jest.mock('fs') -jest.mock('mkdirp') jest.mock('../src/preview') jest.mock('../src/watcher', () => jest.createMockFromModule('../src/watcher')) From 46a19842b6c7975b28bbb81716ad0981d21163f6 Mon Sep 17 00:00:00 2001 From: Yuki Hattori Date: Tue, 12 Apr 2022 23:19:10 +0900 Subject: [PATCH 2/4] Enforce consistent usage of type assertions --- .eslintrc.js | 4 +++ src/server/server-index.ts | 4 +-- src/templates/bespoke/navigation.ts | 4 +-- test/converter.ts | 44 ++++++++++++++--------------- test/marp-cli.ts | 16 +++++------ test/server/server-index.ts | 4 +-- test/theme.ts | 2 +- test/watcher.ts | 14 ++++----- 8 files changed, 48 insertions(+), 44 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index bda50e22..11f574ec 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -31,6 +31,10 @@ module.exports = { '@typescript-eslint/no-explicit-any': 'off', '@typescript-eslint/explicit-function-return-type': 'off', '@typescript-eslint/explicit-module-boundary-types': 'off', + '@typescript-eslint/consistent-type-assertions': [ + 'error', + { assertionStyle: 'as' }, + ], }, }, ], diff --git a/src/server/server-index.ts b/src/server/server-index.ts index 28d5f1a2..abce9e2b 100644 --- a/src/server/server-index.ts +++ b/src/server/server-index.ts @@ -1,8 +1,8 @@ export const showAllKey = 'marp-cli-show-all' export default function serverIndex() { - const showAll = document.getElementById('show-all') - const index = document.getElementById('index') + const showAll = document.getElementById('show-all') as HTMLInputElement + const index = document.getElementById('index') as HTMLElement const applyShowAll = (state: boolean) => { showAll.checked = state diff --git a/src/templates/bespoke/navigation.ts b/src/templates/bespoke/navigation.ts index cc1d976e..5e555bda 100644 --- a/src/templates/bespoke/navigation.ts +++ b/src/templates/bespoke/navigation.ts @@ -68,8 +68,8 @@ const bespokeNavigation = if (elm?.parentElement) detectScrollable(elm.parentElement, dir) } - if (e.deltaX !== 0) detectScrollable(e.target, 'X') - if (e.deltaY !== 0) detectScrollable(e.target, 'Y') + if (e.deltaX !== 0) detectScrollable(e.target as HTMLElement, 'X') + if (e.deltaY !== 0) detectScrollable(e.target as HTMLElement, 'Y') if (scrollable) return e.preventDefault() diff --git a/test/converter.ts b/test/converter.ts index 6ae06f77..ac52f589 100644 --- a/test/converter.ts +++ b/test/converter.ts @@ -58,7 +58,7 @@ describe('Converter', () => { globalDirectives: { theme: 'default' }, imageScale: 2, lang: 'fr', - options: { html: true }, + options: { html: true } as Options, server: false, template: 'test-template', templateOption: {}, @@ -489,8 +489,8 @@ transition: ).rejects.toBeTruthy()) it('converts markdown file and save as html file by default', async () => { - const write = (fs).__mockWriteFile() - await (instance()).convertFile(new File(onePath)) + const write = (fs as any).__mockWriteFile() + await (instance() as any).convertFile(new File(onePath)) expect(write).toHaveBeenCalledWith( `${onePath.slice(0, -3)}.html`, @@ -500,9 +500,9 @@ transition: }) it('converts markdown file and save to specified path when output is defined', async () => { - const write = (fs).__mockWriteFile() + const write = (fs as any).__mockWriteFile() const output = './specified.html' - await (instance({ output })).convertFile(new File(twoPath)) + await (instance({ output }) as any).convertFile(new File(twoPath)) expect(write).toHaveBeenCalledWith( output, @@ -512,11 +512,11 @@ transition: }) it('converts markdown file but not save when output is stdout', async () => { - const write = (fs).__mockWriteFile() + const write = (fs as any).__mockWriteFile() const stdout = jest.spyOn(process.stdout, 'write').mockImplementation() const output = '-' - const ret = await (instance({ output })).convertFile( + const ret = await (instance({ output }) as any).convertFile( new File(threePath) ) @@ -533,7 +533,7 @@ transition: it( 'converts markdown file into PDF', async () => { - const write = (fs).__mockWriteFile() + const write = (fs as any).__mockWriteFile() const opts = { output: 'test.pdf' } const ret = await pdfInstance(opts).convertFile(new File(onePath)) const pdf: Buffer = write.mock.calls[0][1] @@ -551,7 +551,7 @@ transition: it( 'assigns meta info thorugh pdf-lib', async () => { - const write = (fs).__mockWriteFile() + const write = (fs as any).__mockWriteFile() await pdfInstance({ output: 'test.pdf', @@ -580,7 +580,7 @@ transition: async () => { const file = new File(onePath) - const fileCleanup = jest.spyOn(File.prototype, 'cleanup') + const fileCleanup = jest.spyOn(File.prototype as any, 'cleanup') const fileSave = jest .spyOn(File.prototype, 'save') .mockImplementation() @@ -614,7 +614,7 @@ transition: it( 'assigns presenter notes as annotation of PDF', async () => { - const write = (fs).__mockWriteFile() + const write = (fs as any).__mockWriteFile() await pdfInstance({ output: 'test.pdf', @@ -637,7 +637,7 @@ transition: ) it('sets a comment author to notes if set author global directive', async () => { - const write = (fs).__mockWriteFile() + const write = (fs as any).__mockWriteFile() await pdfInstance({ output: 'test.pdf', @@ -673,7 +673,7 @@ transition: let write: jest.Mock beforeEach(() => { - write = (fs).__mockWriteFile() + write = (fs as any).__mockWriteFile() }) const converter = (opts: Partial = {}) => @@ -781,7 +781,7 @@ transition: let write: jest.Mock beforeEach(() => { - write = (fs).__mockWriteFile() + write = (fs as any).__mockWriteFile() }) it( @@ -851,7 +851,7 @@ transition: let write: jest.Mock beforeEach(() => { - write = (fs).__mockWriteFile() + write = (fs as any).__mockWriteFile() }) it( @@ -929,7 +929,7 @@ transition: pages: true, type: ConvertType.png, }) - write = (fs).__mockWriteFile() + write = (fs as any).__mockWriteFile() }) it( @@ -950,9 +950,9 @@ transition: const notesInstance = (opts: Partial = {}) => instance({ ...opts, type: ConvertType.notes }) - const write = (fs).__mockWriteFile() + const write = (fs as any).__mockWriteFile() const output = './specified.txt' - const ret = await (notesInstance({ output })).convertFile( + const ret = await (notesInstance({ output }) as any).convertFile( new File(threePath) ) const notes: Buffer = write.mock.calls[0][1] @@ -969,9 +969,9 @@ transition: const notesInstance = (opts: Partial = {}) => instance({ ...opts, type: ConvertType.notes }) - const write = (fs).__mockWriteFile() + const write = (fs as any).__mockWriteFile() const output = './specified.txt' - const ret = await (notesInstance({ output })).convertFile( + const ret = await (notesInstance({ output }) as any).convertFile( new File(onePath) ) const notes: Buffer = write.mock.calls[0][1] @@ -991,7 +991,7 @@ transition: describe('#convertFiles', () => { describe('with multiple files', () => { it('converts passed files', async () => { - const write = (fs).__mockWriteFile() + const write = (fs as any).__mockWriteFile() await instance().convertFiles([new File(onePath), new File(twoPath)]) expect(write).toHaveBeenCalledTimes(2) @@ -1008,7 +1008,7 @@ transition: ).rejects.toBeInstanceOf(CLIError)) it('converts passed files when output is stdout', async () => { - const write = (fs).__mockWriteFile() + const write = (fs as any).__mockWriteFile() const stdout = jest.spyOn(process.stdout, 'write').mockImplementation() const files = [new File(onePath), new File(twoPath)] diff --git a/test/marp-cli.ts b/test/marp-cli.ts index 0805f773..9f472e7e 100644 --- a/test/marp-cli.ts +++ b/test/marp-cli.ts @@ -224,7 +224,7 @@ describe('Marp CLI', () => { const files = assetFn('_files') let writeFile: jest.Mock - beforeEach(() => (writeFile = (fs).__mockWriteFile())) + beforeEach(() => (writeFile = (fs as any).__mockWriteFile())) it('converts files in specified dir', async () => { jest.spyOn(cli, 'info').mockImplementation() @@ -372,7 +372,7 @@ describe('Marp CLI', () => { info = jest.spyOn(cli, 'info') info.mockImplementation() - ;(fs).__mockWriteFile() + ;(fs as any).__mockWriteFile() }) describe('when passed value is theme name', () => { @@ -395,7 +395,7 @@ describe('Marp CLI', () => { const { css } = (await convert.mock.results[0].value).rendered expect(css).toContain('/* @theme a */') - const converter = convert.mock.instances[0] + const converter: Converter = convert.mock.instances[0] const { themeSet } = converter.options const theme = themeSet.themes.get(cssFile) @@ -441,7 +441,7 @@ describe('Marp CLI', () => { observeSpy = jest.spyOn(ThemeSet.prototype, 'observe') jest.spyOn(cli, 'info').mockImplementation() - ;(fs).__mockWriteFile() + ;(fs as any).__mockWriteFile() }) describe('with specified single file', () => { @@ -555,7 +555,7 @@ describe('Marp CLI', () => { await marpCli(cmd) expect(cvtFiles).toHaveBeenCalled() - return cvtFiles.mock.instances[0] + return cvtFiles.mock.instances[0] as any } finally { cvtFiles.mockRestore() cliInfo.mockRestore() @@ -564,7 +564,7 @@ describe('Marp CLI', () => { it('converts file', async () => { const cliInfo = jest.spyOn(cli, 'info').mockImplementation() - ;(fs).__mockWriteFile() + ;(fs as any).__mockWriteFile() expect(await marpCli([onePath])).toBe(0) @@ -764,7 +764,7 @@ describe('Marp CLI', () => { describe('with -w option', () => { it('starts watching by Watcher.watch()', async () => { jest.spyOn(cli, 'info').mockImplementation() - ;(fs).__mockWriteFile() + ;(fs as any).__mockWriteFile() await runForObservation([onePath, '-w']) expect(Watcher.watch).toHaveBeenCalledWith([onePath], expect.anything()) @@ -1070,7 +1070,7 @@ describe('Marp CLI', () => { .mockResolvedValue(Buffer.from('# markdown')) // reset cached stdin buffer - ;(File).stdinBuffer = undefined + ;(File as any).stdinBuffer = undefined }) it('converts markdown came from stdin and outputs to stdout', async () => { diff --git a/test/server/server-index.ts b/test/server/server-index.ts index 60801ab5..d9118ef6 100644 --- a/test/server/server-index.ts +++ b/test/server/server-index.ts @@ -13,8 +13,8 @@ describe('JavaScript for server index', () => {
    `.trim() - index = document.getElementById('index') - showAll = document.getElementById('show-all') + index = document.getElementById('index') as HTMLUListElement + showAll = document.getElementById('show-all') as HTMLInputElement }) const checkShowAll = (state: boolean) => { diff --git a/test/theme.ts b/test/theme.ts index 30346016..b4f76cdf 100644 --- a/test/theme.ts +++ b/test/theme.ts @@ -75,7 +75,7 @@ describe('ThemeSet', () => { expect(themeSet.onThemeUpdated).toHaveBeenCalledWith('testA2.md') // It does no longer trigger after #unobserve - ;(themeSet.onThemeUpdated).mockClear() + ;(themeSet.onThemeUpdated as jest.Mock).mockClear() themeSet.unobserve('testA.md') await themeSet.load(themeA) diff --git a/test/watcher.ts b/test/watcher.ts index c0b09752..cc2bb3a7 100644 --- a/test/watcher.ts +++ b/test/watcher.ts @@ -33,7 +33,7 @@ describe('Watcher', () => { const file = new File('test.md') const createThemeSet = () => { - const instance = new (ThemeSet)() + const instance = new (ThemeSet as any)() instance.findPath.mockResolvedValue(['test.css']) instance.themes = { delete: jest.fn() } @@ -42,12 +42,12 @@ describe('Watcher', () => { } const createWatcher = (opts: Partial = {}) => - Watcher.watch(['test.md'], { + Watcher.watch(['test.md'], { finder: async () => [file], converter: { convertFiles: jest.fn(), options: { themeSet: createThemeSet() }, - }, + } as any, events: { onConverted: jest.fn(), onError: jest.fn(), @@ -68,7 +68,7 @@ describe('Watcher', () => { expect(notifier.start).toHaveBeenCalled() // Chokidar events - const on = watcher.chokidar.on + const on = watcher.chokidar.on as jest.Mock expect(on).toHaveBeenCalledWith('change', expect.any(Function)) expect(on).toHaveBeenCalledWith('add', expect.any(Function)) @@ -79,8 +79,8 @@ describe('Watcher', () => { const onUnlink = on.mock.calls.find(([e]) => e === 'unlink')[1] // Callbacks - const conv = jest.spyOn(watcher, 'convert').mockImplementation() - const del = jest.spyOn(watcher, 'delete').mockImplementation() + const conv = jest.spyOn(watcher as any, 'convert').mockImplementation() + const del = jest.spyOn(watcher as any, 'delete').mockImplementation() onChange('change') expect(conv).toHaveBeenCalledWith('change') @@ -251,7 +251,7 @@ describe('WatchNotifier', () => { afterEach(() => instance.stop()) - const wss = () => (instance).wss + const wss = () => (instance as any).wss it('starts WebSocket server for notify to HTML', async () => { expect(wss()).toBeUndefined() From 05787259efd2119cfd494894f1496966bb155757 Mon Sep 17 00:00:00 2001 From: Yuki Hattori Date: Tue, 12 Apr 2022 23:34:51 +0900 Subject: [PATCH 3/4] Add test cases about recursive mkdir while saving --- test/converter.ts | 45 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/test/converter.ts b/test/converter.ts index ac52f589..dc484306 100644 --- a/test/converter.ts +++ b/test/converter.ts @@ -20,8 +20,14 @@ import { WatchNotifier } from '../src/watcher' const puppeteerTimeoutMs = 60000 +let mkdirSpy: jest.SpiedFunction + jest.mock('fs') +beforeEach(() => { + mkdirSpy = jest.spyOn(fs.promises, 'mkdir').mockImplementation() +}) + afterAll(() => Converter.closeBrowser()) afterEach(() => jest.restoreAllMocks()) @@ -485,12 +491,12 @@ transition: describe('#convertFile', () => { it('rejects Promise when specified file is not found', () => expect( - (instance() as any).convertFile(new File('_NOT_FOUND_MARKDOWN_')) + instance().convertFile(new File('_NOT_FOUND_MARKDOWN_')) ).rejects.toBeTruthy()) it('converts markdown file and save as html file by default', async () => { const write = (fs as any).__mockWriteFile() - await (instance() as any).convertFile(new File(onePath)) + await instance().convertFile(new File(onePath)) expect(write).toHaveBeenCalledWith( `${onePath.slice(0, -3)}.html`, @@ -502,7 +508,7 @@ transition: it('converts markdown file and save to specified path when output is defined', async () => { const write = (fs as any).__mockWriteFile() const output = './specified.html' - await (instance({ output }) as any).convertFile(new File(twoPath)) + await instance({ output }).convertFile(new File(twoPath)) expect(write).toHaveBeenCalledWith( output, @@ -511,19 +517,40 @@ transition: ) }) + it('tries to create the directory of output file when saving', async () => { + const write = (fs as any).__mockWriteFile() + const output = path.resolve(__dirname, '__test_dir__/out.html') + + await instance({ output }).convertFile(new File(twoPath)) + + expect(write).toHaveBeenCalled() + expect(mkdirSpy).toHaveBeenCalledWith( + path.resolve(__dirname, '__test_dir__'), + { recursive: true } + ) + }) + + it('does not try to create the directory of output file when saving to the root', async () => { + const write = (fs as any).__mockWriteFile() + const output = '/out.html' + + await instance({ output }).convertFile(new File(twoPath)) + + expect(write).toHaveBeenCalled() + expect(mkdirSpy).not.toHaveBeenCalled() + }) + it('converts markdown file but not save when output is stdout', async () => { const write = (fs as any).__mockWriteFile() const stdout = jest.spyOn(process.stdout, 'write').mockImplementation() const output = '-' - const ret = await (instance({ output }) as any).convertFile( - new File(threePath) - ) + const ret = await instance({ output }).convertFile(new File(threePath)) expect(write).not.toHaveBeenCalled() expect(stdout).toHaveBeenCalledTimes(1) expect(ret.file.path).toBe(threePath) - expect(ret.newFile.type).toBe(FileType.StandardIO) + expect(ret.newFile?.type).toBe(FileType.StandardIO) }) describe('when convert type is PDF', () => { @@ -952,7 +979,7 @@ transition: const write = (fs as any).__mockWriteFile() const output = './specified.txt' - const ret = await (notesInstance({ output }) as any).convertFile( + const ret = await notesInstance({ output }).convertFile( new File(threePath) ) const notes: Buffer = write.mock.calls[0][1] @@ -971,7 +998,7 @@ transition: const write = (fs as any).__mockWriteFile() const output = './specified.txt' - const ret = await (notesInstance({ output }) as any).convertFile( + const ret = await notesInstance({ output }).convertFile( new File(onePath) ) const notes: Buffer = write.mock.calls[0][1] From 633352adc22c2a9e0f989aff3b12c7e9b8ac1d8d Mon Sep 17 00:00:00 2001 From: Yuki Hattori Date: Tue, 12 Apr 2022 23:56:42 +0900 Subject: [PATCH 4/4] [ci skip] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3081fedb..3523e3fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Cannot output the conversion result into the drive root ([#442](https://github.com/marp-team/marp-cli/issues/442), [#443](https://github.com/marp-team/marp-cli/pull/443)) + ### Changed - Upgrade Marpit to [v2.2.4](https://github.com/marp-team/marpit/releases/tag/v2.2.4) ([#441](https://github.com/marp-team/marp-cli/pull/441))