Skip to content

Commit

Permalink
Merge pull request #443 from marp-team/recursive-mkdirp-to-the-root
Browse files Browse the repository at this point in the history
Prevent recursive mkdir to the root path while saving
  • Loading branch information
yhatt authored Apr 12, 2022
2 parents c93f562 + 633352a commit f92858b
Show file tree
Hide file tree
Showing 11 changed files with 89 additions and 55 deletions.
4 changes: 4 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
],
},
},
],
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
9 changes: 6 additions & 3 deletions src/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions src/server/server-index.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
export const showAllKey = 'marp-cli-show-all'

export default function serverIndex() {
const showAll = <HTMLInputElement>document.getElementById('show-all')
const index = <HTMLElement>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
Expand Down
4 changes: 2 additions & 2 deletions src/templates/bespoke/navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ const bespokeNavigation =
if (elm?.parentElement) detectScrollable(elm.parentElement, dir)
}

if (e.deltaX !== 0) detectScrollable(<HTMLElement>e.target, 'X')
if (e.deltaY !== 0) detectScrollable(<HTMLElement>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()
Expand Down
3 changes: 0 additions & 3 deletions test/__mocks__/mkdirp.ts

This file was deleted.

79 changes: 53 additions & 26 deletions test/converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,14 @@ import { WatchNotifier } from '../src/watcher'

const puppeteerTimeoutMs = 60000

let mkdirSpy: jest.SpiedFunction<typeof fs.promises.mkdir>

jest.mock('fs')

beforeEach(() => {
mkdirSpy = jest.spyOn(fs.promises, 'mkdir').mockImplementation()
})

afterAll(() => Converter.closeBrowser())
afterEach(() => jest.restoreAllMocks())

Expand Down Expand Up @@ -58,7 +64,7 @@ describe('Converter', () => {
globalDirectives: { theme: 'default' },
imageScale: 2,
lang: 'fr',
options: <Options>{ html: true },
options: { html: true } as Options,
server: false,
template: 'test-template',
templateOption: {},
Expand Down Expand Up @@ -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 = (<any>fs).__mockWriteFile()
await (<any>instance()).convertFile(new File(onePath))
const write = (fs as any).__mockWriteFile()
await instance().convertFile(new File(onePath))

expect(write).toHaveBeenCalledWith(
`${onePath.slice(0, -3)}.html`,
Expand All @@ -500,9 +506,9 @@ transition:
})

it('converts markdown file and save to specified path when output is defined', async () => {
const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()
const output = './specified.html'
await (<any>instance({ output })).convertFile(new File(twoPath))
await instance({ output }).convertFile(new File(twoPath))

expect(write).toHaveBeenCalledWith(
output,
Expand All @@ -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 = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()
const stdout = jest.spyOn(process.stdout, 'write').mockImplementation()

const output = '-'
const ret = await (<any>instance({ output })).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', () => {
Expand All @@ -533,7 +560,7 @@ transition:
it(
'converts markdown file into PDF',
async () => {
const write = (<any>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]
Expand All @@ -551,7 +578,7 @@ transition:
it(
'assigns meta info thorugh pdf-lib',
async () => {
const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()

await pdfInstance({
output: 'test.pdf',
Expand Down Expand Up @@ -580,7 +607,7 @@ transition:
async () => {
const file = new File(onePath)

const fileCleanup = jest.spyOn(<any>File.prototype, 'cleanup')
const fileCleanup = jest.spyOn(File.prototype as any, 'cleanup')
const fileSave = jest
.spyOn(File.prototype, 'save')
.mockImplementation()
Expand Down Expand Up @@ -614,7 +641,7 @@ transition:
it(
'assigns presenter notes as annotation of PDF',
async () => {
const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()

await pdfInstance({
output: 'test.pdf',
Expand All @@ -637,7 +664,7 @@ transition:
)

it('sets a comment author to notes if set author global directive', async () => {
const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()

await pdfInstance({
output: 'test.pdf',
Expand Down Expand Up @@ -673,7 +700,7 @@ transition:
let write: jest.Mock

beforeEach(() => {
write = (<any>fs).__mockWriteFile()
write = (fs as any).__mockWriteFile()
})

const converter = (opts: Partial<ConverterOption> = {}) =>
Expand Down Expand Up @@ -781,7 +808,7 @@ transition:
let write: jest.Mock

beforeEach(() => {
write = (<any>fs).__mockWriteFile()
write = (fs as any).__mockWriteFile()
})

it(
Expand Down Expand Up @@ -851,7 +878,7 @@ transition:
let write: jest.Mock

beforeEach(() => {
write = (<any>fs).__mockWriteFile()
write = (fs as any).__mockWriteFile()
})

it(
Expand Down Expand Up @@ -929,7 +956,7 @@ transition:
pages: true,
type: ConvertType.png,
})
write = (<any>fs).__mockWriteFile()
write = (fs as any).__mockWriteFile()
})

it(
Expand All @@ -950,9 +977,9 @@ transition:
const notesInstance = (opts: Partial<ConverterOption> = {}) =>
instance({ ...opts, type: ConvertType.notes })

const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()
const output = './specified.txt'
const ret = await (<any>notesInstance({ output })).convertFile(
const ret = await notesInstance({ output }).convertFile(
new File(threePath)
)
const notes: Buffer = write.mock.calls[0][1]
Expand All @@ -969,9 +996,9 @@ transition:
const notesInstance = (opts: Partial<ConverterOption> = {}) =>
instance({ ...opts, type: ConvertType.notes })

const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()
const output = './specified.txt'
const ret = await (<any>notesInstance({ output })).convertFile(
const ret = await notesInstance({ output }).convertFile(
new File(onePath)
)
const notes: Buffer = write.mock.calls[0][1]
Expand All @@ -991,7 +1018,7 @@ transition:
describe('#convertFiles', () => {
describe('with multiple files', () => {
it('converts passed files', async () => {
const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()

await instance().convertFiles([new File(onePath), new File(twoPath)])
expect(write).toHaveBeenCalledTimes(2)
Expand All @@ -1008,7 +1035,7 @@ transition:
).rejects.toBeInstanceOf(CLIError))

it('converts passed files when output is stdout', async () => {
const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()
const stdout = jest.spyOn(process.stdout, 'write').mockImplementation()
const files = [new File(onePath), new File(twoPath)]

Expand Down
17 changes: 8 additions & 9 deletions test/marp-cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'))

Expand Down Expand Up @@ -225,7 +224,7 @@ describe('Marp CLI', () => {
const files = assetFn('_files')

let writeFile: jest.Mock
beforeEach(() => (writeFile = (<any>fs).__mockWriteFile()))
beforeEach(() => (writeFile = (fs as any).__mockWriteFile()))

it('converts files in specified dir', async () => {
jest.spyOn(cli, 'info').mockImplementation()
Expand Down Expand Up @@ -373,7 +372,7 @@ describe('Marp CLI', () => {
info = jest.spyOn(cli, 'info')

info.mockImplementation()
;(<any>fs).__mockWriteFile()
;(fs as any).__mockWriteFile()
})

describe('when passed value is theme name', () => {
Expand All @@ -396,7 +395,7 @@ describe('Marp CLI', () => {
const { css } = (await convert.mock.results[0].value).rendered
expect(css).toContain('/* @theme a */')

const converter = <Converter>convert.mock.instances[0]
const converter: Converter = convert.mock.instances[0]
const { themeSet } = converter.options
const theme = themeSet.themes.get(cssFile)

Expand Down Expand Up @@ -442,7 +441,7 @@ describe('Marp CLI', () => {
observeSpy = jest.spyOn(ThemeSet.prototype, 'observe')

jest.spyOn(cli, 'info').mockImplementation()
;(<any>fs).__mockWriteFile()
;(fs as any).__mockWriteFile()
})

describe('with specified single file', () => {
Expand Down Expand Up @@ -556,7 +555,7 @@ describe('Marp CLI', () => {
await marpCli(cmd)
expect(cvtFiles).toHaveBeenCalled()

return <any>cvtFiles.mock.instances[0]
return cvtFiles.mock.instances[0] as any
} finally {
cvtFiles.mockRestore()
cliInfo.mockRestore()
Expand All @@ -565,7 +564,7 @@ describe('Marp CLI', () => {

it('converts file', async () => {
const cliInfo = jest.spyOn(cli, 'info').mockImplementation()
;(<any>fs).__mockWriteFile()
;(fs as any).__mockWriteFile()

expect(await marpCli([onePath])).toBe(0)

Expand Down Expand Up @@ -765,7 +764,7 @@ describe('Marp CLI', () => {
describe('with -w option', () => {
it('starts watching by Watcher.watch()', async () => {
jest.spyOn(cli, 'info').mockImplementation()
;(<any>fs).__mockWriteFile()
;(fs as any).__mockWriteFile()

await runForObservation([onePath, '-w'])
expect(Watcher.watch).toHaveBeenCalledWith([onePath], expect.anything())
Expand Down Expand Up @@ -1071,7 +1070,7 @@ describe('Marp CLI', () => {
.mockResolvedValue(Buffer.from('# markdown'))

// reset cached stdin buffer
;(<any>File).stdinBuffer = undefined
;(File as any).stdinBuffer = undefined
})

it('converts markdown came from stdin and outputs to stdout', async () => {
Expand Down
4 changes: 2 additions & 2 deletions test/server/server-index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ describe('JavaScript for server index', () => {
<ul id="index"></ul>
`.trim()

index = <HTMLUListElement>document.getElementById('index')
showAll = <HTMLInputElement>document.getElementById('show-all')
index = document.getElementById('index') as HTMLUListElement
showAll = document.getElementById('show-all') as HTMLInputElement
})

const checkShowAll = (state: boolean) => {
Expand Down
2 changes: 1 addition & 1 deletion test/theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ describe('ThemeSet', () => {
expect(themeSet.onThemeUpdated).toHaveBeenCalledWith('testA2.md')

// It does no longer trigger after #unobserve
;(<jest.Mock>themeSet.onThemeUpdated).mockClear()
;(themeSet.onThemeUpdated as jest.Mock).mockClear()
themeSet.unobserve('testA.md')

await themeSet.load(themeA)
Expand Down
Loading

0 comments on commit f92858b

Please sign in to comment.