chore: plugin pack command for 2 #916

tasshi-me · 2024-10-24T20:20:35Z

Why

Support plugin manifest v2.

What

refactor plugin module
support manifest v2
add plugin info command

How to test

Manual test example:

## Prepare plugin project to /tmp/hello-kintone

## Build cli-kintone
pnpm install
pnpm build

## Show pack --help
node cli.js plugin pack --help

## Run pack command
node cli.js plugin pack --input /tmp/hello-kintone/src/ --output /tmp/dist/plugin.zip

ls /tmp/dist # => plugin.zip and .ppk exist

## Run pack command with ppk file
node cli.js plugin pack --input /tmp/hello-kintone/src/ --output /tmp/dist/plugin-2.zip --private-key /tmp/dist/*.ppk

ls /tmp/dist # => plugin.zip, plugin-2.zip, and .ppk exist

## Run info command
node cli.js plugin info --input /tmp/dist/plugin.zip # => printing plugin id, name, etc.

Checklist

Read CONTRIBUTING.md
Updated documentation if it is required.
Added/updated tests if it is required. (or tested manually)
Passed pnpm lint and pnpm test on the root directory.

tasshi-me · 2024-10-25T05:28:53Z

前のPRでレビューあったところ

tasshi-me

補足コメント書きました。
かなり大胆にリファクタしたので、↓の概念図も参考にしてください！ 🙏

tasshi-me · 2024-10-28T10:01:00Z

package.json

    "node-rsa": "^1.1.1",
-    "stream-buffers": "^3.0.3",


stream-buffersは Node.js 標準の Utility Consumers に置き換えが案内されていたので置き換えた。
https://www.npmjs.com/package/stream-buffers

tasshi-me · 2024-10-28T10:01:30Z

package.json

@@ -112,13 +110,9 @@
    "chokidar": "^4.0.1",
    "csv-parse": "^4.16.3",
    "csv-stringify": "5.6.5",
-    "debug": "^4.3.7",


debug はcli-kintoneのlogger.debug|traceに置き換えた。

tasshi-me · 2024-10-28T10:03:50Z

package.json

    "https-proxy-agent": "^7.0.5",
    "iconv-lite": "^0.6.3",
-    "mkdirp": "^3.0.1",


fs.promises.mkdir(data, {recursive: true}) に置き換え

tasshi-me · 2024-10-28T10:04:08Z

package.json

@@ -112,13 +110,9 @@
    "chokidar": "^4.0.1",
    "csv-parse": "^4.16.3",
    "csv-stringify": "5.6.5",
-    "debug": "^4.3.7",
-    "execa": "^9.4.1",


使用していたテストを消した。

tasshi-me · 2024-10-28T10:26:07Z

src/plugin/core/crypto/index.ts

+export { PublicKey } from "./public-key";
+export type { PublicKeyInterface } from "./public-key";
+export { PrivateKey } from "./private-key";
+export type { PrivateKeyInterface } from "./private-key";


PublicKey と PrivateKey は変わることは当面なさそうだしInterfaceを切らなくても良さそう。
（アルゴリズムなどが変わるタイミングでIFを切れば良さそう）

tasshi-me · 2024-10-28T12:25:55Z

src/plugin/packer/index.ts

-
-const debug = _debug("packer");
-
-const packer = (


packerの処理はPluginZip.build()に移動した。

tasshi-me · 2024-10-28T12:27:01Z

src/plugin/packer/pack-plugin-from-manifest.ts

-import packer from "./index";
-import { createContentsZip } from "./create-contents-zip";
-
-export const packPluginFromManifest = (


こちらもPluginZip.build()に集約

tasshi-me · 2024-10-28T12:27:38Z

website/docs/guide/commands/plugin-info.md

@@ -0,0 +1,29 @@
+---
+sidebar_position: 1400
+draft: true


draft: trueなのでローカルビルドでした表示されない

tasshi-me · 2024-10-28T16:40:43Z

src/cli/stability.ts

+ * @param stability "experimental" or "deprecated"
+ * @param message additional information
+ */
+export const setStability = <T = {}, U = {}>(


https://github.com/kintone/cli-kintone/pull/915/files#r1815855917 の対応

setStability(cmd, stability, msg) 関数を追加した。
ヘルプメッセージにStabilityが表示され、実行時に警告が出るようになる。

どこかのflagsにstabilityの情報を集約して、それに連動して表示が変わるようにしたかったが、yargsの仕組み上難しそうだった。

コマンドの生成を全てラップするヘルパー関数を置けば実現できるが、既存のレコード系のコマンドにも影響が出るので今回は個別のコマンドをラップする方式にした。

tasshi-me · 2024-10-28T16:48:51Z

src/plugin/packer/cli.ts

-const cli = (pluginDir: string, options_?: Options) => {
+// TODO: Reduce statements in this func
+// eslint-disable-next-line max-statements
+export const run = async (pluginDir: string, options_?: Options) => {


既存のレコード系のコマンドと合わせて関数名を run にした。

ref. https://github.com/kintone/cli-kintone/pull/915/files#r1815859115

めちゃ細かいんですが、ファイル名も（pack.tsとか？）機能に則したものの方が良さそうに思いました。

pack.tsにしました！

shabaraba

@tasshi-me
いくつかコメントしました！
定義がわかりやすくて処理が追いやすかったです！

shabaraba · 2024-10-30T04:34:20Z

src/plugin/core/manifest/interface.ts

+   * Validate Manifest
+   * @param driver If set, also validate file existence and file size.
+   */
+  validate(driver?: DriverInterface): Promise<ValidationResult>;


ManifestStaticInterface.loadJsonFileでdriverを渡している

ManifestInterfaceのメソッドでもdriverをしばしば引数で取る

ので、ManifestInterfaceのプロパティに入れてしまってもいいんじゃないかなと思ったのですがいかがでしょう？

もしくは、driverをメソッドごとに使い分けるユースケースが何かイメージありますでしょうか？

@shabaraba
これも悩んでいた部分ですね！
ManifestがDriverに依存しなくても成立する概念なので、Driverをプロパティに含めていませんでした！

直近は、
・マニフェストを読み込む場所
・マニフェストからファイルの実在を検証する場所
は一緒なので、プロパティに入れてしまっても問題ないと思います。

一方で、今後Driverを使い分けるケースとしては、以下のようなケースを想定しています。
・ZIPファイルやHTTPをソースとして受け付けてローカルにプラグインを生成するパターン
・Webエディタのようなオンメモリ→ZIPファイルを生成するパターン
・Web版plugin-packerのように入力ZIPファイルからプラグインZIPファイルを生成するパターン
　（ZipFileDriverは1つのZIPファイル内のファイル構造を表現しているので、入力・出力で2つのZIPファイルを扱う場合はDriverは2つ必要になります）

自分の考えなのですが、
・ManifestInterface.validate()はManifestの形式的な妥当性だけを検証する
・Driverを使ったファイルの実在確認はContents.validate()を作成する
・ContentsはDriverに依存する（コンストラクタインジェクションする）
とすると見通しが良くなりそうです。

(なんだかんだ言ってメインの利用ケースはLocalFSなので、デフォルト引数をLocalFSにしておいても良さそう)

shabaraba · 2024-10-30T04:49:05Z

src/plugin/core/contents/zip.ts

+/**
+ * Create a zipped contents using stream. Can be faster than createContentsZip above.
+ */
+export const _createContentsZipStream = async (


こちらのメソッドは未使用のようですが、createContentsZipと比較するために用意している、という理解で良いでしょうか？
（あと今後もし使用するのであれば、exportしているのにアンダースコアから始まるのが少し気になりました）

Streamを使ったほうが速度的にもメモリ使用量的にも良さそうなんですけど、まだあまりちゃんと検証していないので置き換えずに置いているような状態です！
一旦はExportしない状態にしました！

shabaraba · 2024-10-30T04:54:03Z

src/plugin/core/plugin/index.ts

+  /**
+   * Create plugin.zip from contents.zip
+   */
+  public static async buildFromContentsZip(


contentsZipではcreateXxxというメソッド名だったので、どちらかの命名に統一できると良さそうに思いました。

またcontentsの方はzip.tsが切り出されているので、構造が（似せれるなら）似ているとよりわかりやすいかなと思いました。

build/generate/createが散らばっているんですが、
~~一旦全てgenerate/generateFromに統一しました。~~
以下の方針で用語調整しました。

build: 複数の何かを組み合わせて何かにする系

generate: 新規に何かを生成する系（privateKeyなど）

createは上記2つに比べて、汎用で、かつCreativeなニュアンスも出そうなので、今回はなくしました

この辺りの単語選びはライブラリごとでも違うので悩ましい、、、
（fs.createReadableStream, crypto.generateKey など）

zip.tsの切り出しも行いました！

shabaraba · 2024-10-30T05:18:33Z

src/plugin/packer/cli.ts

-const cli = (pluginDir: string, options_?: Options) => {
+// TODO: Reduce statements in this func
+// eslint-disable-next-line max-statements
+export const run = async (pluginDir: string, options_?: Options) => {


めちゃ細かいんですが、ファイル名も（pack.tsとか？）機能に則したものの方が良さそうに思いました。

shabaraba · 2024-10-30T05:20:26Z

src/plugin/packer/cli.ts

+  // 5. generate new ppk if not specified
+  const ppkFile = options.ppk;
+  let privateKey: PrivateKey;
+  if (ppkFile) {
+    logger.debug(`loading an existing key: ${ppkFile}`);
+    const ppk = await fs.readFile(ppkFile, "utf8");
+    privateKey = PrivateKey.importKey(ppk);
+  } else {
+    logger.debug("generating a new key");
+    privateKey = PrivateKey.generateKey();
+  }

-/**
- * Load JSON file without caching
- */
-const loadJson = (jsonPath: string) => {
-  const content = fs.readFileSync(jsonPath, "utf8");
-  return JSON.parse(content);
-};
+  const id = privateKey.uuid();
+  logger.debug(`id: ${id}`);


処理の順番に意味がもしなければ、// 7. package plugin.zipで再びこのあたりで定義した変数を利用するので、
// 6. prepare output directoryの後ろにこの処理群を持っていったほうが良さそうに思いましたがいかがでしょうか？

あざます！
処理順番を調整しました。

入力のバリデーション

マニフェストの読み込み

マニフェストの検証

出力ディレクトリの用意

秘密鍵の読み込み、指定されていない場合は生成

プラグインのパッケージング

watchモードの起動（指定されている場合）

プラグインの書き出し

tasshi-me

レビュー対応

tasshi-me self-assigned this Oct 24, 2024

tasshi-me force-pushed the feat/plugin-packer branch from 9c9a9f2 to a8e6a7d Compare October 24, 2024 20:29

tasshi-me force-pushed the feat/plugin-packer-2 branch from 0882ade to d56df1f Compare October 24, 2024 21:25

tasshi-me mentioned this pull request Oct 25, 2024

chore: add plugin pack command (Experimental) #915

Merged

4 tasks

Base automatically changed from feat/plugin-packer to main October 25, 2024 08:15

tasshi-me added 6 commits October 25, 2024 17:26

refactor crypto module

50cc317

add command guide in docs

6fdc82e

use cli-kintone logger instead of console

51c43fb

refactor manifest module

ae959ae

replace callbacks with async await

50eccd2

add description for watch option

5ec52a6

tasshi-me force-pushed the feat/plugin-packer-2 branch from 41f2bc3 to 5ec52a6 Compare October 25, 2024 08:27

tasshi-me added 9 commits October 25, 2024 23:31

extract pluginZip, contentsZip, and manifest modules

78569d4

use ContentsZip class

20c9f66

refactor plugin-zip

e12c6c4

wip

7ee49ad

wip

349c9ac

add trailing line break to ppk file

802d3a9

replace callback with async await

f00fe1d

refactor contents-zip

97837a8

add ManifestStaticInterface

32cafad

tasshi-me force-pushed the feat/plugin-packer-2 branch from c2d994a to 32cafad Compare October 25, 2024 19:26

tasshi-me added 7 commits October 26, 2024 04:33

refactor manifest module

a5de6fb

support manifest v2

c8147c9

rename cli() to run()

ade24d1

fix manifest parsing process

5b5345e

add tests for manifest v2

72e56b2

deprecate packPluginFromManifest

b9a1a4e

fix cli entrypoint

e0506b5

tasshi-me added 5 commits October 28, 2024 00:43

remove execa from deps

dc681dd

delete mkdirp from deps

d4881a8

use cli-kintone logger instead of debug package

2544616

reorder plugin commands

86f7dde

improve error handling on cli

7529e19

tasshi-me force-pushed the feat/plugin-packer-2 branch from 2eb54d1 to 7529e19 Compare October 27, 2024 19:27

tasshi-me added 6 commits October 28, 2024 04:29

update TS target to ES2018

1a9cd24

refactor cli

bb847d3

Merge remote-tracking branch 'origin/main' into feat/plugin-packer-2

8080a6b

delete files generated by tests

50a3e72

add JSDoc

96c0995

add setStability() helper function

3312a0f

tasshi-me force-pushed the feat/plugin-packer-2 branch from 62f1587 to 3312a0f Compare October 28, 2024 16:38

tasshi-me commented Oct 28, 2024

View reviewed changes

tasshi-me marked this pull request as ready for review October 28, 2024 16:54

tasshi-me requested a review from a team as a code owner October 28, 2024 16:54

tasshi-me requested review from chihiro-adachi and shabaraba and removed request for a team October 28, 2024 16:54

shabaraba requested changes Oct 30, 2024

View reviewed changes

tasshi-me added 4 commits November 1, 2024 18:36

reorder run process

99a80b7

fix typo in a comment

68254c1

rename cli to pack

67bffa2

rename create methods to generate or build

95110a4

tasshi-me force-pushed the feat/plugin-packer-2 branch from 39db43e to 95110a4 Compare November 2, 2024 08:33

tasshi-me added 3 commits November 2, 2024 17:41

stop exporting experimental function

aff6ce3

refactor plugin module

1b18965

add comment to private constructors

aacf5bf

tasshi-me commented Nov 2, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into feat/plugin-packer-2

36b94a4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: plugin pack command for 2 #916

chore: plugin pack command for 2 #916

tasshi-me commented Oct 24, 2024 •

edited

Loading

tasshi-me commented Oct 25, 2024 •

edited

Loading

tasshi-me left a comment

tasshi-me Oct 28, 2024

tasshi-me Oct 28, 2024

tasshi-me Oct 28, 2024

tasshi-me Oct 28, 2024

tasshi-me Oct 28, 2024

tasshi-me Oct 28, 2024

tasshi-me Oct 28, 2024

tasshi-me Oct 28, 2024

tasshi-me Oct 28, 2024

tasshi-me Oct 28, 2024

shabaraba Oct 30, 2024

tasshi-me Nov 1, 2024

shabaraba left a comment

shabaraba Oct 30, 2024

tasshi-me Nov 2, 2024

tasshi-me Nov 2, 2024 •

edited

Loading

shabaraba Oct 30, 2024 •

edited

Loading

tasshi-me Nov 2, 2024

shabaraba Oct 30, 2024

tasshi-me Nov 1, 2024 •

edited

Loading

tasshi-me Nov 2, 2024

shabaraba Oct 30, 2024

shabaraba Oct 30, 2024

tasshi-me Nov 1, 2024

tasshi-me left a comment

chore: plugin pack command for 2 #916

Are you sure you want to change the base?

chore: plugin pack command for 2 #916

Conversation

tasshi-me commented Oct 24, 2024 • edited Loading

Why

What

How to test

Checklist

tasshi-me commented Oct 25, 2024 • edited Loading

tasshi-me left a 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

shabaraba left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tasshi-me Nov 2, 2024 • edited Loading

Choose a reason for hiding this comment

shabaraba Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tasshi-me Nov 1, 2024 • edited Loading

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

tasshi-me left a comment

Choose a reason for hiding this comment

tasshi-me commented Oct 24, 2024 •

edited

Loading

tasshi-me commented Oct 25, 2024 •

edited

Loading

tasshi-me Nov 2, 2024 •

edited

Loading

shabaraba Oct 30, 2024 •

edited

Loading

tasshi-me Nov 1, 2024 •

edited

Loading