diff --git a/packages/read-project-manifest/fixtures/commented-package-json5/modified.json5 b/packages/read-project-manifest/fixtures/commented-package-json5/modified.json5 new file mode 100644 index 00000000000..6aaa08ea229 --- /dev/null +++ b/packages/read-project-manifest/fixtures/commented-package-json5/modified.json5 @@ -0,0 +1,9 @@ +/* This is an example of a package.json5 file with comments. */ +{ + /* pnpm should keep comments at the same indentation level */ + name: 'foo', + version: '1.0.0', // it should keep in-line comments on the same line + // It should allow in-line comments with no other content + type: 'commonjs', +} +/* And it should preserve comments at the end of the file. Note no newline. */ \ No newline at end of file diff --git a/packages/read-project-manifest/fixtures/commented-package-json5/package.json5 b/packages/read-project-manifest/fixtures/commented-package-json5/package.json5 new file mode 100644 index 00000000000..c6b65fc12af --- /dev/null +++ b/packages/read-project-manifest/fixtures/commented-package-json5/package.json5 @@ -0,0 +1,9 @@ +/* This is an example of a package.json5 file with comments. */ +{ + /* pnpm should keep comments at the same indentation level */ + name: 'foo', + version: '1.0.0', // it should keep in-line comments on the same line + // It should allow in-line comments with no other content + type: 'module', +} +/* And it should preserve comments at the end of the file. Note no newline. */ \ No newline at end of file diff --git a/packages/read-project-manifest/package.json b/packages/read-project-manifest/package.json index 4a074e53e91..8f5d0128696 100644 --- a/packages/read-project-manifest/package.json +++ b/packages/read-project-manifest/package.json @@ -40,7 +40,8 @@ "parse-json": "^5.2.0", "read-yaml-file": "^2.1.0", "sort-keys": "^4.2.0", - "strip-bom": "^4.0.0" + "strip-bom": "^4.0.0", + "strip-comments-strings": "1.2.0" }, "devDependencies": { "@pnpm/read-project-manifest": "workspace:*", diff --git a/packages/read-project-manifest/src/index.ts b/packages/read-project-manifest/src/index.ts index 12d797032d4..e76c8b7a6e6 100644 --- a/packages/read-project-manifest/src/index.ts +++ b/packages/read-project-manifest/src/index.ts @@ -2,10 +2,11 @@ import { promises as fs, Stats } from 'fs' import path from 'path' import PnpmError from '@pnpm/error' import { ProjectManifest } from '@pnpm/types' -import writeProjectManifest from '@pnpm/write-project-manifest' +import { writeProjectManifest, CommentSpecifier } from '@pnpm/write-project-manifest' import readYamlFile from 'read-yaml-file' import detectIndent from 'detect-indent' +import { parseString, stripComments } from 'strip-comments-strings' import equal from 'fast-deep-equal' import isWindows from 'is-windows' import sortKeys from 'sort-keys' @@ -118,9 +119,55 @@ export async function tryReadProjectManifest (projectDir: string): Promise<{ } function detectFileFormatting (text: string) { + const finalNewline = text.endsWith('\n') + if (!finalNewline) { + /* For the sake of the comment parser, which otherwise loses the + * final character of a final comment + */ + text += '\n' + } + const { comments } = parseString(text) + let stripped = stripComments(text) + if (!finalNewline) { + stripped = stripped.slice(0, -1) + } + let offset = 0 // accumulates difference of indices from text to stripped + for (const comment of comments) { + // Unfortunately, JavaScript lastIndexOf does not have an end parameter: + const preamble: string = stripped.slice(0, comment.index - offset) + const lineStart = Math.max(preamble.lastIndexOf('\n'), 0) + const priorLines = preamble.split('\n') + comment.lineNumber = priorLines.length + if (comment.lineNumber === 1) { + if (preamble.trim().length === 0) { + comment.lineNumber = 0 + } + } else { + comment.after = priorLines[comment.lineNumber - 2] + if (priorLines[0].trim().length === 0) { + /* JSON5.stringify will not have a whitespace-only line at the start */ + comment.lineNumber -= 1 + } + } + let lineEnd: number = stripped.indexOf( + '\n', (lineStart === 0) ? 0 : lineStart + 1) + if (lineEnd < 0) { + lineEnd = stripped.length + } + comment.on = stripped.slice(lineStart, lineEnd) + comment.whitespace = stripped + .slice(lineStart, comment.index - offset) + .match(/^\s*/)[0] + const nextLineEnd = stripped.indexOf('\n', lineEnd + 1) + if (nextLineEnd >= 0) { + comment.before = stripped.slice(lineEnd, nextLineEnd) + } + offset += comment.indexEnd - comment.index + } return { + comments, indent: detectIndent(text).indent, - insertFinalNewline: text.endsWith('\n'), + insertFinalNewline: finalNewline, } } @@ -174,6 +221,7 @@ async function readPackageYaml (filePath: string) { function createManifestWriter ( opts: { initialManifest: ProjectManifest + comments?: CommentSpecifier[] indent?: string | number | undefined insertFinalNewline?: boolean manifestPath: string @@ -184,6 +232,7 @@ function createManifestWriter ( updatedManifest = normalize(updatedManifest) if (force === true || !equal(initialManifest, updatedManifest)) { await writeProjectManifest(opts.manifestPath, updatedManifest, { + comments: opts.comments, indent: opts.indent, insertFinalNewline: opts.insertFinalNewline, }) diff --git a/packages/read-project-manifest/src/types/strip-comments-strings/index.d.ts b/packages/read-project-manifest/src/types/strip-comments-strings/index.d.ts new file mode 100644 index 00000000000..5fda8942278 --- /dev/null +++ b/packages/read-project-manifest/src/types/strip-comments-strings/index.d.ts @@ -0,0 +1,2 @@ +// strip-comments-strings/index.d.ts +declare module 'strip-comments-strings'; diff --git a/packages/read-project-manifest/test/index.ts b/packages/read-project-manifest/test/index.ts index fb6a296a569..6e4fa186abd 100644 --- a/packages/read-project-manifest/test/index.ts +++ b/packages/read-project-manifest/test/index.ts @@ -82,6 +82,26 @@ test('preserve space indentation in json5 file', async () => { expect(rawManifest).toBe("{\n name: 'foo',\n dependencies: {\n bar: '1.0.0',\n },\n}\n") }) +test('preserve comments in json5 file', async () => { + const originalManifest = await fs.readFile( + path.join(fixtures, 'commented-package-json5/package.json5'), 'utf8') + const modifiedManifest = await fs.readFile( + path.join(fixtures, 'commented-package-json5/modified.json5'), 'utf8') + + process.chdir(tempy.directory()) + await fs.writeFile('package.json5', originalManifest, 'utf8') + + const { manifest, writeProjectManifest } = await readProjectManifest(process.cwd()) + + // Have to make a change to get it to write anything: + const newManifest = Object.assign({}, manifest, { type: 'commonjs' }) + + await writeProjectManifest(newManifest) + + const resultingManifest = await fs.readFile('package.json5', 'utf8') + expect(resultingManifest).toBe(modifiedManifest) +}) + test('do not save manifest if it had no changes', async () => { process.chdir(tempy.directory()) diff --git a/packages/write-project-manifest/src/index.ts b/packages/write-project-manifest/src/index.ts index c4964b0826f..a5d0ea40ac4 100644 --- a/packages/write-project-manifest/src/index.ts +++ b/packages/write-project-manifest/src/index.ts @@ -10,10 +10,21 @@ const YAML_FORMAT = { noRefs: true, } -export default async function writeProjectManifest ( +export interface CommentSpecifier { + type: string + content: string + lineNumber: number + after?: string + on: string + whitespace: string + before?: string +} + +export async function writeProjectManifest ( filePath: string, manifest: ProjectManifest, opts?: { + comments?: CommentSpecifier[] indent?: string | number | undefined insertFinalNewline?: boolean } @@ -26,8 +37,90 @@ export default async function writeProjectManifest ( await fs.mkdir(path.dirname(filePath), { recursive: true }) const trailingNewline = opts?.insertFinalNewline === false ? '' : '\n' - const json = (fileType === 'json5' ? JSON5 : JSON) + let json = (fileType === 'json5' ? JSON5 : JSON) .stringify(manifest, undefined, opts?.indent ?? '\t') + if (opts?.comments) { + // We need to reintroduce the comments. So create an index of + // the lines of the manifest so we can try to match them up. + // We eliminate whitespace and quotes because pnpm may have changed them. + const jsonLines = json.split('\n') + const index = {} + for (let i = 0; i < jsonLines.length; ++i) { + const key = jsonLines[i].replace(/[\s'"]/g, '') + if (key in index) { + index[key] = -1 + } else { + index[key] = i + } + } + + // A place to put comments that come _before_ the lines they are + // anchored to: + const jsonPrefix: Record = {} + for (const comment of opts.comments) { + // First if we can find the line the comment was on, that is + // the most reliable locator: + let key = comment.on.replace(/[\s'"]/g, '') + if (key && index[key] !== undefined && index[key] >= 0) { + jsonLines[index[key]] += ' ' + comment.content + continue + } + // Next, if it's not before anything, it must have been at the very end: + if (comment.before === undefined) { + jsonLines[jsonLines.length - 1] += comment.whitespace + comment.content + continue + } + // Next, try to put it before something; note the comment extractor + // used the convention that position 0 is before the first line: + let location = (comment.lineNumber === 0) ? 0 : -1 + if (location < 0) { + key = comment.before.replace(/[\s'"]/g, '') + if (key && index[key] !== undefined) { + location = index[key] + } + } + if (location >= 0) { + if (jsonPrefix[location]) { + jsonPrefix[location] += ' ' + comment.content + } else { + const inlineWhitespace = comment.whitespace.startsWith('\n') + ? comment.whitespace.slice(1) + : comment.whitespace + jsonPrefix[location] = inlineWhitespace + comment.content + } + continue + } + // The last definite indicator we can use is that it is after something: + if (comment.after) { + key = comment.after.replace(/[\s'"]/g, '') + if (key && index[key] !== undefined && index[key] >= 0) { + jsonLines[index[key]] += comment.whitespace + comment.content + continue + } + } + // Finally, try to get it in the right general location by using the + // line number, but warn the user the comment may have been relocated: + location = comment.lineNumber - 1 // 0 was handled above + let separator = ' ' + if (location >= jsonLines.length) { + location = jsonLines.length - 1 + separator = '\n' + } + jsonLines[location] += separator + comment.content + + ' /* [comment possibly relocated by pnpm] */' + } + // Insert the accumulated prefixes: + for (let i = 0; i < jsonLines.length; ++i) { + if (jsonPrefix[i]) { + jsonLines[i] = jsonPrefix[i] + '\n' + jsonLines[i] + } + } + // And reassemble the manifest: + json = jsonLines.join('\n') + } + return writeFileAtomic(filePath, `${json}${trailingNewline}`) } + +export default writeProjectManifest diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 478d75b049d..d403437be9c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4587,6 +4587,9 @@ importers: strip-bom: specifier: ^4.0.0 version: 4.0.0 + strip-comments-strings: + specifier: 1.2.0 + version: 1.2.0 devDependencies: '@pnpm/read-project-manifest': specifier: workspace:* @@ -14815,6 +14818,10 @@ packages: resolution: {integrity: sha512-3xurFv5tEgii33Zi8Jtp55wEIILR9eh34FAW00PZf+JnSsTmV/ioewSgQl97JHvgjoRGwPShsWm+IdrxB35d0w==} engines: {node: '>=8'} + /strip-comments-strings/1.2.0: + resolution: {integrity: sha512-zwF4bmnyEjZwRhaak9jUWNxc0DoeKBJ7lwSN/LEc8dQXZcUFG6auaaTQJokQWXopLdM3iTx01nQT8E4aL29DAQ==} + dev: false + /strip-final-newline/2.0.0: resolution: {integrity: sha512-BrpvfNAE3dcvq7ll3xVumzjKjZQ5tI1sEUIKr3Uoks0XUl45St3FlatVqef9prk4jRDzhW6WZg+3bk93y6pLjA==} engines: {node: '>=6'}