From 4f2540b3eabbea035ec2724a6598bc50a4944857 Mon Sep 17 00:00:00 2001 From: Kunal Kundu Date: Sat, 28 Jan 2023 18:39:43 +0530 Subject: [PATCH 1/7] feat: support comments in vscode settings --- npm-shrinkwrap.json | 63 +++++++++++++++++++++++++++++++++ package.json | 1 + src/recipes/vscode/settings.mjs | 10 +++--- 3 files changed, 69 insertions(+), 5 deletions(-) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 4fc29fa2fba..abf503f790f 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -30,6 +30,7 @@ "ci-info": "^3.0.0", "clean-deep": "^3.0.2", "commander": "^9.2.0", + "comment-json": "^4.2.3", "concordance": "^5.0.0", "configstore": "^5.0.0", "content-type": "^1.0.4", @@ -6589,6 +6590,11 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/array-timsort": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/array-timsort/-/array-timsort-1.0.3.tgz", + "integrity": "sha512-/+3GRL7dDAGEfM6TseQk/U+mi18TU2Ms9I3UlLdUMhz2hbvGNTKdj9xniwXfUqgYhHxRx0+8UnKkvlNwVU+cWQ==" + }, "node_modules/array-union": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/array-union/-/array-union-2.1.0.tgz", @@ -8467,6 +8473,26 @@ "node": "^12.20.0 || >=14" } }, + "node_modules/comment-json": { + "version": "4.2.3", + "resolved": "https://registry.npmjs.org/comment-json/-/comment-json-4.2.3.tgz", + "integrity": "sha512-SsxdiOf064DWoZLH799Ata6u7iV658A11PlWtZATDlXPpKGJnbJZ5Z24ybixAi+LUUqJ/GKowAejtC5GFUG7Tw==", + "dependencies": { + "array-timsort": "^1.0.3", + "core-util-is": "^1.0.3", + "esprima": "^4.0.1", + "has-own-prop": "^2.0.0", + "repeat-string": "^1.6.1" + }, + "engines": { + "node": ">= 6" + } + }, + "node_modules/comment-json/node_modules/core-util-is": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/core-util-is/-/core-util-is-1.0.3.tgz", + "integrity": "sha512-ZQBvi1DcpJ4GDqanjucZ2Hj3wEO5pZDS89BWbkcrvdxksJorwUDDZamX9ldFkp9aw2lmBDLgkObEA4DWNJ9FYQ==" + }, "node_modules/common-path-prefix": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/common-path-prefix/-/common-path-prefix-3.0.0.tgz", @@ -13799,6 +13825,14 @@ "node": ">=0.10.0" } }, + "node_modules/has-own-prop": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/has-own-prop/-/has-own-prop-2.0.0.tgz", + "integrity": "sha512-Pq0h+hvsVm6dDEa8x82GnLSYHOzNDt7f0ddFa3FqcQlgzEiptPqL+XrOJNavjOzSYiYWIrgeVYYgGlLmnxwilQ==", + "engines": { + "node": ">=8" + } + }, "node_modules/has-symbol-support-x": { "version": "1.4.2", "resolved": "https://registry.npmjs.org/has-symbol-support-x/-/has-symbol-support-x-1.4.2.tgz", @@ -28884,6 +28918,11 @@ "is-string": "^1.0.7" } }, + "array-timsort": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/array-timsort/-/array-timsort-1.0.3.tgz", + "integrity": "sha512-/+3GRL7dDAGEfM6TseQk/U+mi18TU2Ms9I3UlLdUMhz2hbvGNTKdj9xniwXfUqgYhHxRx0+8UnKkvlNwVU+cWQ==" + }, "array-union": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/array-union/-/array-union-2.1.0.tgz", @@ -30273,6 +30312,25 @@ "resolved": "https://registry.npmjs.org/commander/-/commander-9.5.0.tgz", "integrity": "sha512-KRs7WVDKg86PWiuAqhDrAQnTXZKraVcCc6vFdL14qrZ/DcWwuRo7VoiYXalXO7S5GKpqYiVEwCbgFDfxNHKJBQ==" }, + "comment-json": { + "version": "4.2.3", + "resolved": "https://registry.npmjs.org/comment-json/-/comment-json-4.2.3.tgz", + "integrity": "sha512-SsxdiOf064DWoZLH799Ata6u7iV658A11PlWtZATDlXPpKGJnbJZ5Z24ybixAi+LUUqJ/GKowAejtC5GFUG7Tw==", + "requires": { + "array-timsort": "^1.0.3", + "core-util-is": "^1.0.3", + "esprima": "^4.0.1", + "has-own-prop": "^2.0.0", + "repeat-string": "^1.6.1" + }, + "dependencies": { + "core-util-is": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/core-util-is/-/core-util-is-1.0.3.tgz", + "integrity": "sha512-ZQBvi1DcpJ4GDqanjucZ2Hj3wEO5pZDS89BWbkcrvdxksJorwUDDZamX9ldFkp9aw2lmBDLgkObEA4DWNJ9FYQ==" + } + } + }, "common-path-prefix": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/common-path-prefix/-/common-path-prefix-3.0.0.tgz", @@ -34420,6 +34478,11 @@ } } }, + "has-own-prop": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/has-own-prop/-/has-own-prop-2.0.0.tgz", + "integrity": "sha512-Pq0h+hvsVm6dDEa8x82GnLSYHOzNDt7f0ddFa3FqcQlgzEiptPqL+XrOJNavjOzSYiYWIrgeVYYgGlLmnxwilQ==" + }, "has-symbol-support-x": { "version": "1.4.2", "resolved": "https://registry.npmjs.org/has-symbol-support-x/-/has-symbol-support-x-1.4.2.tgz", diff --git a/package.json b/package.json index d7b0e232f59..aa688101fd7 100644 --- a/package.json +++ b/package.json @@ -95,6 +95,7 @@ "ci-info": "^3.0.0", "clean-deep": "^3.0.2", "commander": "^9.2.0", + "comment-json": "^4.2.3", "concordance": "^5.0.0", "configstore": "^5.0.0", "content-type": "^1.0.4", diff --git a/src/recipes/vscode/settings.mjs b/src/recipes/vscode/settings.mjs index 509e4a3759a..01ff52729bd 100644 --- a/src/recipes/vscode/settings.mjs +++ b/src/recipes/vscode/settings.mjs @@ -1,17 +1,17 @@ import { mkdir, readFile, stat, writeFile } from 'fs/promises' import { dirname, relative } from 'path' +import CommentJSON from 'comment-json' import unixify from 'unixify' export const applySettings = (existingSettings, { denoBinary, edgeFunctionsPath, repositoryRoot }) => { const relativeEdgeFunctionsPath = unixify(relative(repositoryRoot, edgeFunctionsPath)) - const settings = { - ...existingSettings, + const settings = CommentJSON.assign(existingSettings, { 'deno.enable': true, 'deno.enablePaths': existingSettings['deno.enablePaths'] || [], 'deno.unstable': true, 'deno.importMap': '.netlify/edge-functions-import-map.json', - } + }) // If the Edge Functions path isn't already in `deno.enabledPaths`, let's add // it. @@ -42,7 +42,7 @@ export const getSettings = async (settingsPath) => { return { fileExists: true, - settings: JSON.parse(file), + settings: CommentJSON.parse(file.toString()), } } catch (error) { if (error.code !== 'ENOENT') { @@ -57,7 +57,7 @@ export const getSettings = async (settingsPath) => { } export const writeSettings = async ({ settings, settingsPath }) => { - const serializedSettings = JSON.stringify(settings, null, 2) + const serializedSettings = CommentJSON.stringify(settings, null, 2) await mkdir(dirname(settingsPath), { recursive: true }) await writeFile(settingsPath, serializedSettings) From 2f778c1893d2df200c8aab507505e9a0bf0d67d6 Mon Sep 17 00:00:00 2001 From: Kunal Kundu Date: Mon, 30 Jan 2023 22:55:01 +0530 Subject: [PATCH 2/7] refactor: destructure imports --- src/recipes/vscode/settings.mjs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/recipes/vscode/settings.mjs b/src/recipes/vscode/settings.mjs index 01ff52729bd..7810ddc15c1 100644 --- a/src/recipes/vscode/settings.mjs +++ b/src/recipes/vscode/settings.mjs @@ -1,12 +1,12 @@ import { mkdir, readFile, stat, writeFile } from 'fs/promises' import { dirname, relative } from 'path' -import CommentJSON from 'comment-json' +import { parse, assign, stringify } from 'comment-json' import unixify from 'unixify' export const applySettings = (existingSettings, { denoBinary, edgeFunctionsPath, repositoryRoot }) => { const relativeEdgeFunctionsPath = unixify(relative(repositoryRoot, edgeFunctionsPath)) - const settings = CommentJSON.assign(existingSettings, { + const settings = assign(existingSettings, { 'deno.enable': true, 'deno.enablePaths': existingSettings['deno.enablePaths'] || [], 'deno.unstable': true, @@ -42,7 +42,7 @@ export const getSettings = async (settingsPath) => { return { fileExists: true, - settings: CommentJSON.parse(file.toString()), + settings: parse(file.toString()), } } catch (error) { if (error.code !== 'ENOENT') { @@ -57,7 +57,7 @@ export const getSettings = async (settingsPath) => { } export const writeSettings = async ({ settings, settingsPath }) => { - const serializedSettings = CommentJSON.stringify(settings, null, 2) + const serializedSettings = stringify(settings, null, 2) await mkdir(dirname(settingsPath), { recursive: true }) await writeFile(settingsPath, serializedSettings) From 452dea2886f3bbcc2e1932ab10686f2f890065c2 Mon Sep 17 00:00:00 2001 From: Kunal Kundu Date: Sat, 4 Feb 2023 22:40:37 +0530 Subject: [PATCH 3/7] test: add test to check if JSON with comments is handled --- .../integration/640.command.recipes.test.cjs | 46 ++++++++++++++++++- tests/integration/utils/handle-questions.cjs | 2 + 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/tests/integration/640.command.recipes.test.cjs b/tests/integration/640.command.recipes.test.cjs index 668de5decf6..b98df228a3c 100644 --- a/tests/integration/640.command.recipes.test.cjs +++ b/tests/integration/640.command.recipes.test.cjs @@ -2,11 +2,12 @@ const { readFile } = require('fs/promises') const { resolve } = require('path') const test = require('ava') +const { parse } = require('comment-json') const execa = require('execa') const callCli = require('./utils/call-cli.cjs') const cliPath = require('./utils/cli-path.cjs') -const { CONFIRM, answerWithValue, handleQuestions } = require('./utils/handle-questions.cjs') +const { CONFIRM, NO, answerWithValue, handleQuestions } = require('./utils/handle-questions.cjs') const { withSiteBuilder } = require('./utils/site-builder.cjs') const { normalize } = require('./utils/snapshots.cjs') @@ -96,3 +97,46 @@ test('Does not generate a new VS Code settings file if the user does not confirm t.is(error.code, 'ENOENT') }) }) + +test.only('Handles JSON with comments', async (t) => { + await withSiteBuilder('repo', async (builder) => { + const comment = '// sets value for someSetting' + await builder + .withContentFile({ + path: '.vscode/settings.json', + content: `{ + "deno.enablePaths":["/some/path"], + "someSetting":"value" ${comment} + }`, + }) + .buildAsync() + + const childProcess = execa(cliPath, ['recipes', 'vscode'], { + cwd: builder.directory, + }) + const settingsPath = resolve(builder.directory, '.vscode', 'settings.json') + + handleQuestions(childProcess, [ + { + question: `There is a VS Code settings file at ${settingsPath}. Can we update it?`, + answer: answerWithValue(CONFIRM), + }, + { + question: 'The Deno VS Code extension is recommended. Would you like to install it now?', + answer: answerWithValue(NO), + }, + ]) + + await childProcess + + const settingsText = await readFile(`${builder.directory}/.vscode/settings.json`, { encoding: 'utf8' }) + t.log(settingsText) + t.true(settingsText.includes(comment)) + + const settings = parse(settingsText, null, true) + t.is(settings.someSetting, 'value') + t.is(settings['deno.enable'], true) + t.is(settings['deno.importMap'], '.netlify/edge-functions-import-map.json') + t.deepEqual([...settings['deno.enablePaths']], ['/some/path', 'netlify/edge-functions']) + }) +}) diff --git a/tests/integration/utils/handle-questions.cjs b/tests/integration/utils/handle-questions.cjs index 51a68aad466..7ce20747ae5 100644 --- a/tests/integration/utils/handle-questions.cjs +++ b/tests/integration/utils/handle-questions.cjs @@ -27,10 +27,12 @@ const answerWithValue = (value) => `${value}${CONFIRM}` const CONFIRM = '\n' const DOWN = '\u001B[B' +const NO = 'n' module.exports = { handleQuestions, answerWithValue, CONFIRM, DOWN, + NO, } From 1dbee1a07106b2e4cab7a0f5b170a33ca8f48704 Mon Sep 17 00:00:00 2001 From: Kunal Kundu Date: Sat, 4 Feb 2023 23:00:00 +0530 Subject: [PATCH 4/7] fix: remove unnecessary log statement --- tests/integration/640.command.recipes.test.cjs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/640.command.recipes.test.cjs b/tests/integration/640.command.recipes.test.cjs index b98df228a3c..a5678961e02 100644 --- a/tests/integration/640.command.recipes.test.cjs +++ b/tests/integration/640.command.recipes.test.cjs @@ -130,7 +130,6 @@ test.only('Handles JSON with comments', async (t) => { await childProcess const settingsText = await readFile(`${builder.directory}/.vscode/settings.json`, { encoding: 'utf8' }) - t.log(settingsText) t.true(settingsText.includes(comment)) const settings = parse(settingsText, null, true) From b0c3a086da64c889fc963e42dc82fde30f60b822 Mon Sep 17 00:00:00 2001 From: Kunal Kundu Date: Mon, 6 Feb 2023 16:29:42 +0530 Subject: [PATCH 5/7] test: remove .only modifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Eduardo Bouças --- tests/integration/640.command.recipes.test.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/640.command.recipes.test.cjs b/tests/integration/640.command.recipes.test.cjs index a5678961e02..729453e7e5b 100644 --- a/tests/integration/640.command.recipes.test.cjs +++ b/tests/integration/640.command.recipes.test.cjs @@ -98,7 +98,7 @@ test('Does not generate a new VS Code settings file if the user does not confirm }) }) -test.only('Handles JSON with comments', async (t) => { +test('Handles JSON with comments', async (t) => { await withSiteBuilder('repo', async (builder) => { const comment = '// sets value for someSetting' await builder From 26ec300c9088ed5ba3ea3bff98fd1f1e27ca4d00 Mon Sep 17 00:00:00 2001 From: Kunal Kundu Date: Mon, 6 Feb 2023 16:38:16 +0530 Subject: [PATCH 6/7] refactor: use utf8 encoding in readFile instead of using toString on contents --- src/recipes/vscode/settings.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/recipes/vscode/settings.mjs b/src/recipes/vscode/settings.mjs index 7810ddc15c1..57fa7de10d1 100644 --- a/src/recipes/vscode/settings.mjs +++ b/src/recipes/vscode/settings.mjs @@ -38,11 +38,11 @@ export const getSettings = async (settingsPath) => { throw new Error(`${settingsPath} is not a valid file.`) } - const file = await readFile(settingsPath) + const file = await readFile(settingsPath, 'utf8') return { fileExists: true, - settings: parse(file.toString()), + settings: parse(file), } } catch (error) { if (error.code !== 'ENOENT') { From 04bef8d1103e83e46281d51a1ed8a4ea30f6e4bc Mon Sep 17 00:00:00 2001 From: Kunal Kundu Date: Thu, 9 Feb 2023 22:30:42 +0530 Subject: [PATCH 7/7] refactor: import comment-json using a namespace --- src/recipes/vscode/settings.mjs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/recipes/vscode/settings.mjs b/src/recipes/vscode/settings.mjs index 57fa7de10d1..90bd3a74fcd 100644 --- a/src/recipes/vscode/settings.mjs +++ b/src/recipes/vscode/settings.mjs @@ -1,12 +1,13 @@ import { mkdir, readFile, stat, writeFile } from 'fs/promises' import { dirname, relative } from 'path' -import { parse, assign, stringify } from 'comment-json' +// eslint-disable-next-line import/no-namespace +import * as JSONC from 'comment-json' import unixify from 'unixify' export const applySettings = (existingSettings, { denoBinary, edgeFunctionsPath, repositoryRoot }) => { const relativeEdgeFunctionsPath = unixify(relative(repositoryRoot, edgeFunctionsPath)) - const settings = assign(existingSettings, { + const settings = JSONC.assign(existingSettings, { 'deno.enable': true, 'deno.enablePaths': existingSettings['deno.enablePaths'] || [], 'deno.unstable': true, @@ -42,7 +43,7 @@ export const getSettings = async (settingsPath) => { return { fileExists: true, - settings: parse(file), + settings: JSONC.parse(file), } } catch (error) { if (error.code !== 'ENOENT') { @@ -57,7 +58,7 @@ export const getSettings = async (settingsPath) => { } export const writeSettings = async ({ settings, settingsPath }) => { - const serializedSettings = stringify(settings, null, 2) + const serializedSettings = JSONC.stringify(settings, null, 2) await mkdir(dirname(settingsPath), { recursive: true }) await writeFile(settingsPath, serializedSettings)