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.

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

feat: patch dotenv file parse #14116

Open
wants to merge 2 commits into
base: 3.x
Choose a base branch
from

Conversation

PengBoUESTC
Copy link
Contributor

这个 PR 做了什么? (简要描述所做更改)

调整 dotenv 文件解析逻辑,支持 根据打包平台 读取配置文件

这个 PR 是什么类型? (至少选择一个)

  • 错误修复(Bugfix) issue: fix #
  • 新功能(Feature)
  • 代码重构(Refactor)
  • TypeScript 类型定义修改(Typings)
  • 文档修改(Docs)
  • 代码风格更新(Code style update)
  • 其他,请描述(Other, please describe):

这个 PR 涉及以下平台:

  • 所有小程序
  • 微信小程序
  • 支付宝小程序
  • 百度小程序
  • 字节跳动小程序
  • QQ 轻应用
  • 京东小程序
  • 快应用平台(QuickApp)
  • Web 平台(H5)
  • 移动端(React-Native)
  • 鸿蒙(harmony)

@bigmeow
Copy link
Member

bigmeow commented Jul 5, 2023

  1. 修改了后测试用例没过
  2. 大概说一下你希望的用法和优先级顺序

@@ -59,7 +59,7 @@ export default class CLI {
}
const mode = args.mode || process.env.NODE_ENV
// 这里解析 dotenv 以便于 config 解析时能获取 dotenv 配置信息
const expandEnv = dotenvParse(appPath, args.envPrefix, mode)
const expandEnv = dotenvParse(appPath, args as unknown as Parameters<typeof dotenvParse>[1])
Copy link
Member

@bigmeow bigmeow Jul 5, 2023

Choose a reason for hiding this comment

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

mode 默认值还是要的, 函数里面可以不用加 if mode 的判断

@bigmeow
Copy link
Member

bigmeow commented Jul 5, 2023

我梳理了一下,之前的优先级(优先级从上到下代表从高到低):
.env.[mode].local
.env.[mode]
.env.local
.env

如果除了 mode 以外,再加个 type 进来, 这个维度似乎就有些复杂了,
理论上新增 type 后的优先级:
.env.[mode].[type].local // 新增
.env.[mode].local

.env.[mode].[type] // 新增
.env.[mode]

.env.[type].local // 新增
.env.local

.env.[type] // 新增
.env

使用难度瞬间拔高,上述列的最后六行的优先级,我列出来了但是感觉似乎还是有点顺序问题...

思路一

我的建议是按文件夹分组, type 就是文件夹的名称, 每个文件夹下保持原来的 env 读取规则:

weapp
    .env.[mode].local`
    .env.[mode]  
    .env.local
    .env
alipay
    .env.[mode].local`
    .env.[mode]  
    .env.local
    .env
-

这样设计一目了然.
如果检测到用户使用了多个 文件夹来分组 env 文件, 则只读取指定文件夹内的 env 文件, 外部的不读取。

思路二:

当检测到 env 文件名包含类型名称时,则只读取包含 type 名称的 env文件, 其他的文件都不读取,假设有下面几个 env 文件:

.env.weapp.dev.local`
.env.weapp.dev
.env.weapp.local
.env.weapp

.env.h5.dev.local`
.env.h5.dev  
.env.h5.local
.env.h5

当编译微信小程序时, 则只读取文件名.env.weapp 开头的 env 文件, 其他 env 都不读取. 此时文件优先级为:

    .env.[type].[mode].local`
    .env.[type].[mode]  
    .env.[type].local
    .env.[type]

和以前优先级一样, 只是多了个 type

@PengBoUESTC
Copy link
Contributor Author

我梳理了一下,之前的优先级(优先级从上到下代表从高到低): .env.[mode].local .env.[mode] .env.local .env

如果除了 mode 以外,再加个 type 进来, 这个维度似乎就有些复杂了, 理论上新增 type 后的优先级: .env.[mode].[type].local // 新增 .env.[mode].local

.env.[mode].[type] // 新增 .env.[mode]

.env.[type].local // 新增 .env.local

.env.[type] // 新增 .env

使用难度瞬间拔高,上述列的最后六行的优先级,我列出来了但是感觉似乎还是有点顺序问题...

思路一

我的建议是按文件夹分组, type 就是文件夹的名称, 每个文件夹下保持原来的 env 读取规则:

weapp
    .env.[mode].local`
    .env.[mode]  
    .env.local
    .env
alipay
    .env.[mode].local`
    .env.[mode]  
    .env.local
    .env
-

这样设计一目了然. 如果检测到用户使用了多个 文件夹来分组 env 文件, 则只读取指定文件夹内的 env 文件, 外部的不读取。

思路二:

当检测到 env 文件名包含类型名称时,则只读取包含 type 名称的 env文件, 其他的文件都不读取,假设有下面几个 env 文件:

.env.weapp.dev.local`
.env.weapp.dev
.env.weapp.local
.env.weapp

.env.h5.dev.local`
.env.h5.dev  
.env.h5.local
.env.h5

当编译微信小程序时, 则只读取文件名.env.weapp 开头的 env 文件, 其他 env 都不读取. 此时文件优先级为:

    .env.[type].[mode].local`
    .env.[type].[mode]  
    .env.[type].local
    .env.[type]

和以前优先级一样, 只是多了个 type

嗯, 加了 平台 配置文件就会成倍增加,分文件夹其实不能解决这个问题,但是可能更清晰;第二种方案的问题是 公共 常量配置 没法维护。我想想

@PengBoUESTC
Copy link
Contributor Author

@bigmeow 把配置文件 都放到 env 文件夹下面, 这样内部再分 平台,文件的 优先级顺序都是同样的, 只是指定平台的 配置文件 优先级,比未指定平台的配置文件优先级高。这样 env 外层的配置文件 可以写 公共配置,平台自己的写在自己的文件夹里;

@bigmeow
Copy link
Member

bigmeow commented Jul 6, 2023

  • 无论怎么改,老的得兼容(通用约定俗称), 新的你可以单独放单独文件夹,或者有个参数可以指定放哪个文件夹
  • 公共 常量配置 没法维护 这个简单,在思路二的基础上,比如约定都放 .env 文件里,或者约定个名字叫.env.common
  • 复杂度一定不能太高,不然还不如 copy

@ZEJIA-LIU ZEJIA-LIU changed the base branch from main to 3.x July 16, 2024 06:52
@ZEJIA-LIU ZEJIA-LIU deleted the branch NervJS:3.x July 16, 2024 07:15
@ZEJIA-LIU ZEJIA-LIU closed this Jul 16, 2024
@ZEJIA-LIU ZEJIA-LIU reopened this Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants