Skip to content

Commit

Permalink
feat(pkg-manifest): preserve comments in json5 manifests (#5677)
Browse files Browse the repository at this point in the history
  Use npm package `strip-comments-strings` to find all comments in any
  manifest file as it is read. Save them as part of the "file formatting"
  detected by the manifest reader, noting for each the text of the lines
  they are on, before, and after, and the line number as a fallback.

  When the manifest is written, attempt to place each comment back in
  the resulting JSON5 text, so that the text of the line it is on,
  before, or after (in that priority order) matches the text at time of
  reading. Otherwise, so that no comments are lost, replace the comment
  on the same line number, adding a notation that it may have been
  relocated (due to sorting dependencies, for example, it may no longer
  be in the same "logical position" in the file, even though it is on
  the same physical line number).

  When comments are in fairly ordinary positions and the manifest does
  not change too drastically (i.e. once the dependencies are sorted as
  pnpm prefers, and there are not many parameters added all at once),
  this strategy results in exact preservation of the comments, as a new
  test shows.

  The motivation for this commit is to take advantage of the feature of
  JSON5 that it allows human-readable comments. For this feature to be
  useful in the case of package.json5 manifests, those comments must be
  preserved across manifest changes.

  Partially resolves #2008. [That issue requests comment preservation
  also for YAML manifests, but I have no experience working with YAML,
  and it also requests that key order be preserved, but I did not
  address key order because current code in the the pnpm manifest
  reader/writer _explicitly_ reorders keys -- clearly deliberately --
  so I did not want to simply remove code that appeared to have been
  purposefully written and included.]

Co-authored-by: Zoltan Kochan <z@kochan.io>
  • Loading branch information
gwhitney and zkochan committed Nov 27, 2022
1 parent 868f2fb commit 0d12d38
Show file tree
Hide file tree
Showing 24 changed files with 395 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changeset/famous-ants-lie.md
@@ -0,0 +1,5 @@
---
"@pnpm/text.comments-parser": major
---

Initial release.
7 changes: 7 additions & 0 deletions .changeset/strong-houses-visit.md
@@ -0,0 +1,7 @@
---
"@pnpm/read-project-manifest": minor
"@pnpm/write-project-manifest": minor
"pnpm": patch
---

Comments in `package.json5` are preserver [#2008](https://github.com/pnpm/pnpm/issues/2008).
23 changes: 23 additions & 0 deletions __typings__/typed.d.ts
Expand Up @@ -45,6 +45,29 @@ declare module 'split-cmd' {
export function splitToObject (cmd: string): { command: string, args: string[] }
}

declare module 'strip-comments-strings' {
export interface CodeItem {
// What feature of the code has been found:
type: string,
// The indices of the feature in the original code string:
index: number,
indexEnd: number,
// The test of the feature
content: string
}
export interface CodeAttributes {
// The remaining code text after all features have been stripped:
text: string,
// The items found:
comments: CodeItem[],
regexes: CodeItem[],
strings: CodeItem[]
}
export function parseString (str: string): CodeAttributes;
export type CodeItemReplacer = (item: CodeItem) => string;
export function stripComments (
str: string, replacer?: CodeItemReplacer): string;
}

declare module 'bin-links/lib/fix-bin' {
function fixBin (path: string, execMode: number): Promise<void>;
Expand Down
@@ -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. */
@@ -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. */
1 change: 1 addition & 0 deletions pkg-manifest/read-project-manifest/package.json
Expand Up @@ -32,6 +32,7 @@
"@gwhitney/detect-indent": "7.0.1",
"@pnpm/error": "workspace:*",
"@pnpm/graceful-fs": "workspace:*",
"@pnpm/text.comments-parser": "workspace:*",
"@pnpm/types": "workspace:*",
"@pnpm/write-project-manifest": "workspace:*",
"fast-deep-equal": "^3.1.3",
Expand Down
17 changes: 14 additions & 3 deletions pkg-manifest/read-project-manifest/src/index.ts
Expand Up @@ -2,9 +2,9 @@ import { promises as fs, Stats } from 'fs'
import path from 'path'
import { PnpmError } from '@pnpm/error'
import { ProjectManifest } from '@pnpm/types'
import { extractComments, CommentSpecifier } from '@pnpm/text.comments-parser'
import { writeProjectManifest } from '@pnpm/write-project-manifest'
import readYamlFile from 'read-yaml-file'

import detectIndent from '@gwhitney/detect-indent'
import equal from 'fast-deep-equal'
import isWindows from 'is-windows'
Expand Down Expand Up @@ -76,7 +76,7 @@ export async function tryReadProjectManifest (projectDir: string): Promise<{
fileName: 'package.json5',
manifest: data,
writeProjectManifest: createManifestWriter({
...detectFileFormatting(text),
...detectFileFormattingAndComments(text),
initialManifest: data,
manifestPath,
}),
Expand Down Expand Up @@ -117,6 +117,15 @@ export async function tryReadProjectManifest (projectDir: string): Promise<{
}
}

function detectFileFormattingAndComments (text: string) {
const { comments, text: newText, hasFinalNewline } = extractComments(text)
return {
comments,
indent: detectIndent(newText).indent,
insertFinalNewline: hasFinalNewline,
}
}

function detectFileFormatting (text: string) {
return {
indent: detectIndent(text).indent,
Expand All @@ -143,7 +152,7 @@ export async function readExactProjectManifest (manifestPath: string) {
return {
manifest: data,
writeProjectManifest: createManifestWriter({
...detectFileFormatting(text),
...detectFileFormattingAndComments(text),
initialManifest: data,
manifestPath,
}),
Expand Down Expand Up @@ -174,6 +183,7 @@ async function readPackageYaml (filePath: string) {
function createManifestWriter (
opts: {
initialManifest: ProjectManifest
comments?: CommentSpecifier[]
indent?: string | number | undefined
insertFinalNewline?: boolean
manifestPath: string
Expand All @@ -184,6 +194,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,
})
Expand Down
20 changes: 20 additions & 0 deletions pkg-manifest/read-project-manifest/test/index.ts
Expand Up @@ -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())

Expand Down
3 changes: 3 additions & 0 deletions pkg-manifest/read-project-manifest/tsconfig.json
Expand Up @@ -18,6 +18,9 @@
{
"path": "../../packages/types"
},
{
"path": "../../text/comments-parser"
},
{
"path": "../write-project-manifest"
}
Expand Down
1 change: 1 addition & 0 deletions pkg-manifest/write-project-manifest/package.json
Expand Up @@ -29,6 +29,7 @@
},
"homepage": "https://github.com/pnpm/pnpm/blob/main/pkg-manifest/write-project-manifest#readme",
"dependencies": {
"@pnpm/text.comments-parser": "workspace:*",
"@pnpm/types": "workspace:*",
"json5": "^2.2.1",
"write-file-atomic": "^4.0.2",
Expand Down
18 changes: 16 additions & 2 deletions pkg-manifest/write-project-manifest/src/index.ts
@@ -1,5 +1,6 @@
import { promises as fs } from 'fs'
import path from 'path'
import { insertComments, CommentSpecifier } from '@pnpm/text.comments-parser'
import { ProjectManifest } from '@pnpm/types'
import JSON5 from 'json5'
import writeFileAtomic from 'write-file-atomic'
Expand All @@ -14,6 +15,7 @@ export async function writeProjectManifest (
filePath: string,
manifest: ProjectManifest,
opts?: {
comments?: CommentSpecifier[]
indent?: string | number | undefined
insertFinalNewline?: boolean
}
Expand All @@ -25,9 +27,21 @@ export async function writeProjectManifest (

await fs.mkdir(path.dirname(filePath), { recursive: true })
const trailingNewline = opts?.insertFinalNewline === false ? '' : '\n'
const indent = opts?.indent ?? '\t'

const json = (fileType === 'json5' ? JSON5 : JSON)
.stringify(manifest, undefined, opts?.indent ?? '\t')
const json = (
fileType === 'json5'
? stringifyJson5(manifest, indent, opts?.comments)
: JSON.stringify(manifest, undefined, indent)
)

return writeFileAtomic(filePath, `${json}${trailingNewline}`)
}

function stringifyJson5 (obj: object, indent: string | number, comments?: CommentSpecifier[]) {
const json5 = JSON5.stringify(obj, undefined, indent)
if (comments) {
return insertComments(json5, comments)
}
return json5
}
3 changes: 3 additions & 0 deletions pkg-manifest/write-project-manifest/tsconfig.json
Expand Up @@ -11,6 +11,9 @@
"references": [
{
"path": "../../packages/types"
},
{
"path": "../../text/comments-parser"
}
]
}
21 changes: 21 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pnpm-workspace.yaml
Expand Up @@ -20,6 +20,7 @@ packages:
- resolving/*
- reviewing/*
- store/*
- text/*
- workspace/*
- "!**/example/**"
- "!**/test/**"
Expand Down
14 changes: 14 additions & 0 deletions text/comments-parser/README.md
@@ -0,0 +1,14 @@
# @pnpm/text.comments-parser

> Extracts and inserts comments from/to text
## Installation

```
pnpm i @pnpm/text.comments-parser
```

## License

[MIT](LICENSE)
1 change: 1 addition & 0 deletions text/comments-parser/jest.config.js
@@ -0,0 +1 @@
module.exports = require('../../jest.config.js')
41 changes: 41 additions & 0 deletions text/comments-parser/package.json
@@ -0,0 +1,41 @@
{
"name": "@pnpm/text.comments-parser",
"description": "Extracts and inserts comments from/to text",
"version": "0.0.0",
"bugs": {
"url": "https://github.com/pnpm/pnpm/issues"
},
"main": "lib/index.js",
"types": "lib/index.d.ts",
"files": [
"lib",
"!*.map"
],
"keywords": [
"pnpm7"
],
"license": "MIT",
"engines": {
"node": ">=14.6"
},
"repository": "https://github.com/pnpm/pnpm/blob/main/text/comments-parser",
"scripts": {
"start": "tsc --watch",
"_test": "jest",
"test": "pnpm run compile && pnpm run _test",
"lint": "eslint src/**/*.ts test/**/*.ts",
"prepublishOnly": "pnpm run compile",
"compile": "tsc --build && pnpm run lint --fix"
},
"dependencies": {
"strip-comments-strings": "1.2.0"
},
"homepage": "https://github.com/pnpm/pnpm/blob/main/text/comments-parser#readme",
"funding": "https://opencollective.com/pnpm",
"devDependencies": {
"@pnpm/text.comments-parser": "workspace:*"
},
"exports": {
".": "./lib/index.js"
}
}
9 changes: 9 additions & 0 deletions text/comments-parser/src/CommentSpecifier.ts
@@ -0,0 +1,9 @@
export interface CommentSpecifier {
type: string
content: string
lineNumber: number
after?: string
on: string
whitespace: string
before?: string
}

0 comments on commit 0d12d38

Please sign in to comment.