feat: generate package lock file #63

akitaSummer · 2023-12-02T16:08:14Z

No description provided.

elrrrrrrr · 2023-12-07T05:47:23Z

packages/cli/lib/util.js

@@ -600,9 +601,25 @@ exports.readPkgJSON = async function readPkgJSON(cwd) {
  return { pkg, pkgPath };
 };

-exports.readPackageLock = async function readPackageLock(cwd) {
+exports.readPackageLock = async function readPackageLock(cwd, noPackageLock) {


独立一个方法来做 ensure 逻辑？

elrrrrrrr · 2023-12-07T05:48:25Z

packages/cli/lib/util.js

+      try {
+        await fs.stat(lockPath);
+      } catch {
+        const arb = new Arborist({


可能还需要额外传一下 workspaces 相关参数。
直接用子进程调 npm 客户端来生成？

elrrrrrrr · 2023-12-07T05:49:17Z

packages/cli/lib/index.js

@@ -18,7 +18,7 @@ const { MirrorConfig } = require('binary-mirror-config');
 // 有依赖树（package-lock.json）走 npm / npminstall 极速安装
 exports.install = async options => {
  options.env = util.getEnv(options.env, options.args);
-  const { packageLock } = options.packageLock || (await util.readPackageLock(options.cwd));
+  const { packageLock } = options.packageLock || (await util.readPackageLock(options.cwd, options.noPackageLock));


修改了默认值逻辑，会有一个不兼容变更 🤔，需要发 0.4 了

是的，这是个大版本改动

killagu · 2023-12-13T04:57:52Z

packages/cli/bin/rapid.js

@@ -32,12 +32,17 @@ const argv = yargs
          describe: 'Dependency types to omit from the installation tree on disk',
          type: 'array',
          default: [],
+        })
+        .option('no-package-lock', {


这是 npm 的标准参数吗？
另外一般行为不会加 no 前缀，这是命令行解析工具自己会做的。 --no-package-lock 等价于 --package-lock=false

killagu · 2023-12-13T14:59:01Z

packages/cli/lib/index.js

@@ -18,6 +18,11 @@ const { MirrorConfig } = require('binary-mirror-config');
 // 有依赖树（package-lock.json）走 npm / npminstall 极速安装
 exports.install = async options => {
  options.env = util.getEnv(options.env, options.args);
+
+  if (!options.noPackageLock) {


这里是不是也该成 options.packageLock 比较好，尽量不使用双重否定。

packageLock字段在下面第26行被占用了

那换个字段名称吧，避开 no 这种情况。

killagu · 2023-12-13T14:59:45Z

packages/cli/lib/util.js

+exports.generatePackageLock = async cwd => {
+  try {
+    const lockPath = path.join(cwd || exports.findLocalPrefix(), './package-lock.json');
+    const isExist = await fs.stat(lockPath).then(() => true).catch(() => false);


在 AsyncFunction 中使用 then/catch 不是一个好主意。

这个仅仅是用于判断文件是否存在，如果再套层try catch，会显得有点回掉地狱的感觉

try/catch 不嵌套是正确的。可以用过把 try/catch 拆小来解决这个问题。

let isExist = true; try { await fs.stat(lockPath); } catch { isExist = false; } ... generate package lock

最好的实践是 try/catch 仅捕获一个错误。

killagu · 2023-12-13T15:00:58Z

packages/cli/lib/util.js

+    const isExist = await fs.stat(lockPath).then(() => true).catch(() => false);
+    if (!isExist) {
+      console.log('npm install --force --package-lock-only --ignore-scripts is running');
+      const { stdout, stderr } = await exec('npm install --force --package-lock-only --ignore-scripts', { cwd });


为什么需要使用 --force? 另外 npm install 时还有大量额外参数是不是应该一起透传才符合直觉？

使用force是为了尽量能生成，我们需要传递这么多参数吗，我们只是一个安装器，并不是一个npm cli吧

killagu · 2023-12-13T15:01:25Z

packages/cli/lib/util.js

+    if (!isExist) {
+      console.log('npm install --force --package-lock-only --ignore-scripts is running');
+      const { stdout, stderr } = await exec('npm install --force --package-lock-only --ignore-scripts', { cwd });
+      console.log(`${stdout}`);


这样 stdio 不如直接 inherit？

使用了execSync的inherit

killagu · 2023-12-13T15:02:25Z

packages/cli/lib/util.js

+    const isExist = await fs.stat(lockPath).then(() => true).catch(() => false);
+    if (!isExist) {
+      console.log('npm install --force --package-lock-only --ignore-scripts is running');
+      const { stdout, stderr } = await exec('npm install --force --package-lock-only --ignore-scripts', { cwd });


使用子进程并不是一个好主意，在开发 cli 时需要很注意子进程的生命周期，如果父进程退出了，子进程未退出会造成 shell 的异常。

改成了execSync是不是就不需要注意这个了呢

需要去封装一下 cp.exec 方法，注意退出。

killagu · 2023-12-13T15:05:00Z

packages/cli/test/package_lock.test.js

+      mm.restore();
+    });
+
+    it('should run all project installation scripts', async () => {


不要只写集成测试。这样各种边界 case 是未测试的。

killagu · 2023-12-13T15:05:19Z

packages/cli/test/package_lock.test.js

+  describe('not exist lock file', async () => {
+    let fixture;
+    beforeEach(async () => {
+      mm(process.env, NYDUS_CSI_ROOT_ENV, 'true');


怎么会有这个环境变量？

feat: generate package lock file

9c0eb08

akitaSummer force-pushed the feat/generta_package_lock branch from 8a66d8f to 9c0eb08 Compare December 2, 2023 16:14

fix: fix test

74c2750

akitaSummer force-pushed the feat/generta_package_lock branch from f651c7d to 74c2750 Compare December 4, 2023 05:41

elrrrrrrr reviewed Dec 7, 2023

View reviewed changes

akitaSummer force-pushed the feat/generta_package_lock branch 8 times, most recently from b81b97a to c7df817 Compare December 12, 2023 14:43

fix: use child_process generate package-lock.json

86f621f

akitaSummer force-pushed the feat/generta_package_lock branch from c7df817 to 86f621f Compare December 12, 2023 14:47

killagu reviewed Dec 13, 2023

View reviewed changes

akitaSummer force-pushed the feat/generta_package_lock branch 4 times, most recently from 045ed49 to cd0c7fd Compare December 13, 2023 05:56

fix: add package-lock option

b1b1e72

akitaSummer force-pushed the feat/generta_package_lock branch from cd0c7fd to b1b1e72 Compare December 13, 2023 06:01

killagu reviewed Dec 13, 2023

View reviewed changes

akitaSummer added 3 commits December 19, 2023 00:15

fix: use execSync

1caf4fd

fix: del env in test

5000f4c

fix: use execa handle npm install

69e7e7a

elrrrrrrr merged commit 29e0f82 into master Feb 1, 2024
14 checks passed

elrrrrrrr deleted the feat/generta_package_lock branch February 1, 2024 09:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: generate package lock file #63

feat: generate package lock file #63

akitaSummer commented Dec 2, 2023

elrrrrrrr Dec 7, 2023

akitaSummer Dec 13, 2023

elrrrrrrr Dec 7, 2023

akitaSummer Dec 13, 2023

elrrrrrrr Dec 7, 2023

akitaSummer Dec 13, 2023

killagu Dec 13, 2023

akitaSummer Dec 13, 2023

killagu Dec 13, 2023

akitaSummer Dec 18, 2023

killagu Dec 19, 2023

killagu Dec 13, 2023

akitaSummer Dec 18, 2023

killagu Dec 19, 2023

akitaSummer Dec 25, 2023

killagu Dec 13, 2023

akitaSummer Dec 18, 2023

killagu Dec 13, 2023

akitaSummer Dec 18, 2023

killagu Dec 13, 2023

akitaSummer Dec 18, 2023

killagu Dec 19, 2023

akitaSummer Dec 25, 2023

killagu Dec 13, 2023

killagu Dec 13, 2023

akitaSummer Dec 18, 2023

feat: generate package lock file #63

feat: generate package lock file #63

Conversation

akitaSummer commented Dec 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment