Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: easy config #468

Merged
merged 2 commits into from
May 21, 2023
Merged

feat: easy config #468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9588136
feat: easy config
elrrrrrrr May 21, 2023
1737c7f
f
fengmk2 May 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion INTEGRATE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,21 @@
}
```

3. 修改 `config.default.ts` 文件，可以直接复制 cnpmcore 中的内容
3. 修改 `config.default.ts` 文件，可以直接覆盖默认配置
```typescript
import { SyncMode } from 'cnpmcore/common/constants';
import { cnpmcoreConfig } from 'cnpmcore/common/config';

export default () => {
const config = {};
config.cnpmcore = {
...cnpmcoreConfig,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

集成时应当手动对 cnpmcore 进行赋值，目前 mergeConfig 时不会进行deepMerge。如果 cnpmcoreConfig 内新增了默认配置，而应用没有修改，会丢失配置。

enableChangesStream: false,
syncMode: SyncMode.all,
};
return config;
}
```

### 🧑‍🤝‍🧑 集成 cnpmcore

elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
10 changes: 10 additions & 0 deletions app/common/typing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { cnpmcoreConfig } from '../port/config';
import { Readable } from 'stream';
import { IncomingHttpHeaders } from 'http';
import { EggContext } from '@eggjs/tegg';
Expand Down Expand Up @@ -62,3 +63,12 @@ export interface AuthClient {
getAuthUrl(ctx: EggContext): Promise<AuthUrlResult>;
ensureCurrentUser(): Promise<userResult | null>;
}

declare module 'egg' {
// eslint-disable-next-line
// @ts-ignore
// avoid TS2310 Type 'EggAppConfig' recursively references itself as a base type.
interface EggAppConfig {
cnpmcore: typeof cnpmcoreConfig;
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这段代码通过添加导入语句，扩展了AuthClient接口，并在EggAppConfig中添加了cnpmcore属性。我们无法进行完整的审核因为缺少上下文信息，但是根据这个补丁本身，这里有几点建议：

  • 检查config.ts文件或其他相关的文件，确保cnpmcoreConfig被正确地实现和使用。
  • 这个补丁中添加了一些注释屏蔽了TS2310错误，如果可能的话，最好避免在代码中使用这样的注释。
  • 确认这个补丁是否对代码库中其他部分造成了影响。

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

会触发循环引用 bug，用 ts-ignore 可以避免编译直接输出到 d.ts 😈

如果去除的话，需要手动在应用集成时手动赋值。

config.cnpmcore = {
  ...cnpmcoreConfig,
} as typeof cnpmcoreConfig;

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这段代码加入了对cnpmcoreConfig的导入，并且定义了一个EggAppConfig的接口，
将cnpmcoreConfig设置为Egg应用程序配置的一部分。没有看到与原始代码的比较，难以确定是否存在风险或其它建议。

1 change: 1 addition & 0 deletions app/port/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { cnpmcoreConfig } from '../../config/config.default';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这段代码补丁很简单，主要是将一个名为“cnpmcoreConfig”的导出对象从默认配置文件中导出。在没有更多上下文的情况下，很难评估此代码是否存在任何错误风险或改进建议。但是，如果你能提供更多信息，那么我就可以更好地帮助你进行代码审查。

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

代码补丁的作用是将cnpmcoreConfig从文件config/config.default中导出。根据这段代码本身来看，没有发现明显的风险或错误。如果需要更全面的代码审查，请提供更多相关代码信息。至于改进建议，需要更多背景知识才能给出具体的意见。

2 changes: 1 addition & 1 deletion app/port/controller/HomeController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type LegacyInfo = {

type SiteEnvInfo = {
sync_model: string;
sync_binary: string;
sync_binary: boolean;
instance_start_time: Date;
node_version: string;
app_version: string;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个代码提供了一个类型别名 LegacyInfo 和一个类型别名 SiteEnvInfo，其中 SiteEnvInfo 包括了一个名为 sync_binary 的字段，类型是由字符串改为了布尔值。代码的更改似乎没有引入任何风险。

有一个改进建议：命名约定中规定应该在布尔值变量名前添加动词 is 或者 has，这样做可以让代码更易读懂，比如 has_sync_binary 或者 is_sync_enabled。

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这段代码改动较小，主要是将 sync_binary 的类型从字符串改为布尔值。没有明显的风险和潜在的错误。

不过，可以考虑以下几点改进：

  1. 建议使用更加明确的命名，例如 is_sync_binary_enabled 或者 has_sync_binary等，增强可读性和易用性。
  2. 由于 sync_model 和 sync_binary 是相关联的，建议在代码中增加一些注释，来表达两者之间的关系和含义。

Expand Down
235 changes: 153 additions & 82 deletions config/config.default.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,91 +5,162 @@ import OSSClient from 'oss-cnpm';
import { patchAjv } from '../app/port/typebox';
import { SyncDeleteMode, SyncMode } from '../app/common/constants';

export const cnpmcoreConfig = {
name: 'cnpm',
/**
* enable hook or not
*/
hookEnable: false,
/**
* mac custom hooks count
*/
hooksLimit: 20,
/**
* upstream registry url
*/
sourceRegistry: 'https://registry.npmjs.org',
/**
* upstream registry is base on `cnpmcore` or not
* if your upstream is official npm registry, please turn it off
*/
sourceRegistryIsCNpm: false,
/**
* sync upstream first
*/
syncUpstreamFirst: false,
/**
* sync upstream timeout, default is 3mins
*/
sourceRegistrySyncTimeout: 180000,
/**
* sync task high water size, default is 100
*/
taskQueueHighWaterSize: 100,
/**
* sync mode
* - none: don't sync npm package
* - admin: don't sync npm package,only admin can create sync task by sync contorller.
* - all: sync all npm packages
* - exist: only sync exist packages, effected when `enableCheckRecentlyUpdated` or `enableChangesStream` is enabled
*/
syncMode: SyncMode.none,
syncDeleteMode: SyncDeleteMode.delete,
syncPackageWorkerMaxConcurrentTasks: 10,
triggerHookWorkerMaxConcurrentTasks: 10,
createTriggerHookWorkerMaxConcurrentTasks: 10,
/**
* stop syncing these packages in future
*/
syncPackageBlockList: [] as string[],
/**
* check recently from https://www.npmjs.com/browse/updated, if use set changesStreamRegistry to cnpmcore,
* maybe you should disable it
*/
enableCheckRecentlyUpdated: true,
/**
* mirror binary, default is false
*/
enableSyncBinary: false,
/**
* sync binary source api, default is `${sourceRegistry}/-/binary`
*/
syncBinaryFromAPISource: '',
/**
* enable sync downloads data from source registry https://github.com/cnpm/cnpmcore/issues/108
* all three parameters must be configured at the same time to take effect
*/
enableSyncDownloadData: false,
syncDownloadDataSourceRegistry: '',
/**
* should be YYYY-MM-DD format
*/
syncDownloadDataMaxDate: '',
/**
* @see https://github.com/npm/registry-follower-tutorial
*/
enableChangesStream: false,
checkChangesStreamInterval: 500,
changesStreamRegistry: 'https://replicate.npmjs.com',
/**
* handle _changes request mode, default is 'streaming', please set it to 'json' when on cnpmcore registry
*/
changesStreamRegistryMode: 'streaming',
/**
* registry url
*/
registry: 'http://localhost:7001',
/**
* https://docs.npmjs.com/cli/v6/using-npm/config#always-auth npm <= 6
* if `alwaysAuth=true`, all api request required access token
*/
alwaysAuth: false,
/**
* white scope list
*/
allowScopes: [
'@cnpm',
'@cnpmcore',
'@example',
],
/**
* allow publish non-scope package, disable by default
*/
allowPublishNonScopePackage: false,
/**
* Public registration is allowed, otherwise only admins can login
*/
allowPublicRegistration: true,
/**
* default system admins
*/
admins: {
// name: email
cnpmcore_admin: 'admin@cnpmjs.org',
},
/**
* use webauthn for login, https://webauthn.guide/
* only support platform authenticators, browser support: https://webauthn.me/browser-support
*/
enableWebAuthn: false,
/**
* http response cache control header
*/
enableCDN: false,
/**
* if you are using CDN, can override it
* it meaning cache 300s on CDN server and client side.
*/
cdnCacheControlHeader: 'public, max-age=300',
/**
* if you are using CDN, can set it to 'Accept, Accept-Encoding'
*/
cdnVaryHeader: 'Accept, Accept-Encoding',
/**
* store full package version manifests data to database table(package_version_manifests), default is false
*/
enableStoreFullPackageVersionManifestsToDatabase: false,
/**
* only support npm as client and npm >= 7.0.0 allow publish action
*/
enableNpmClientAndVersionCheck: true,
/**
* sync when package not found, only effect when syncMode = all/exist
*/
syncNotFound: false,
/**
* redirect to source registry when package not found
*/
redirectNotFound: true,
/**
* enable unpkg features, https://github.com/cnpm/cnpmcore/issues/452
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elrrrrrrr 我将注释改成可以提示的
image

*/
enableUnpkg: true,
};

export default (appInfo: EggAppConfig) => {
const config = {} as PowerPartial<EggAppConfig>;

config.cnpmcore = {
name: 'cnpm',
hooksLimit: 20,
sourceRegistry: 'https://registry.npmjs.org',
// upstream registry is base on `cnpmcore` or not
// if your upstream is official npm registry, please turn it off
sourceRegistryIsCNpm: false,
syncUpstreamFirst: false,
// 3 mins
sourceRegistrySyncTimeout: 180000,
taskQueueHighWaterSize: 100,
// sync mode
// - none: don't sync npm package
// - admin: don't sync npm package,only admin can create sync task by sync contorller.
// - all: sync all npm packages
// - exist: only sync exist packages, effected when `enableCheckRecentlyUpdated` or `enableChangesStream` is enabled
syncMode: SyncMode.none,
syncDeleteMode: SyncDeleteMode.delete,
hookEnable: false,
syncPackageWorkerMaxConcurrentTasks: 10,
triggerHookWorkerMaxConcurrentTasks: 10,
createTriggerHookWorkerMaxConcurrentTasks: 10,
// stop syncing these packages in future
syncPackageBlockList: [],
// check recently from https://www.npmjs.com/browse/updated, if use set changesStreamRegistry to cnpmcore,
// maybe you should disable it
enableCheckRecentlyUpdated: true,
// mirror binary, default is false
enableSyncBinary: false,
// cnpmcore api: https://r.cnpmjs.org/-/binary
syncBinaryFromAPISource: '',
// enable sync downloads data from source registry https://github.com/cnpm/cnpmcore/issues/108
// all three parameters must be configured at the same time to take effect
enableSyncDownloadData: false,
syncDownloadDataSourceRegistry: '',
syncDownloadDataMaxDate: '', // should be YYYY-MM-DD format
// https://github.com/npm/registry-follower-tutorial
enableChangesStream: false,
checkChangesStreamInterval: 500,
changesStreamRegistry: 'https://replicate.npmjs.com',
// handle _changes request mode, default is 'streaming', please set it to 'json' when on cnpmcore registry
changesStreamRegistryMode: 'streaming',
registry: 'http://localhost:7001',
// https://docs.npmjs.com/cli/v6/using-npm/config#always-auth npm <= 6
// if `alwaysAuth=true`, all api request required access token
alwaysAuth: false,
// white scope list
allowScopes: [
'@cnpm',
'@cnpmcore',
'@example',
],
// allow publish non-scope package, disable by default
allowPublishNonScopePackage: false,
// Public registration is allowed, otherwise only admins can login
allowPublicRegistration: true,
// default system admins
admins: {
// name: email
cnpmcore_admin: 'admin@cnpmjs.org',
},
// use webauthn for login, https://webauthn.guide/
// only support platform authenticators, browser support: https://webauthn.me/browser-support
enableWebAuthn: false,
// http response cache control header
enableCDN: false,
// if you are using CDN, can override it
// it meaning cache 300s on CDN server and client side.
cdnCacheControlHeader: 'public, max-age=300',
// if you are using CDN, can set it to 'Accept, Accept-Encoding'
cdnVaryHeader: 'Accept, Accept-Encoding',
// store full package version manifests data to database table(package_version_manifests), default is false
enableStoreFullPackageVersionManifestsToDatabase: false,
// only support npm as client and npm >= 7.0.0 allow publish action
enableNpmClientAndVersionCheck: true,
// sync when package not found, only effect when syncMode = all/exist
syncNotFound: false,
// redirect to source registry when package not found
redirectNotFound: true,
// enable unpkg features, https://github.com/cnpm/cnpmcore/issues/452
enableUnpkg: true,
};
config.cnpmcore = cnpmcoreConfig;

// override config from framework / plugin
config.dataDir = join(appInfo.root, '.cnpmcore');
Expand Down