Conversation
nighca
commented
Sep 18, 2017
nighca
commented
- 重构 CLI 工具实现，添加更多 option & command
- image 的逻辑中也走 CLI 工具，保持一致性
- 添加更多 debug 信息
- debug extract-style 的功能
add global ENV config eg: fec-builder -e staing add fec-builder production build eg: fec-builder --build
|
|
|
done |
| }, | ||
| ENV_VARIABLES_FILE: { | ||
| alias: 'f', | ||
| desc: 'Target file path for env variables', |
There was a problem hiding this comment.
这个我感觉不用吧，路径支持绝对路径跟 ./xxx 这种相对路径，相对路径是相对于 cwd，跟普遍的行为是一致的
There was a problem hiding this comment.
还是说 envVariables 的格式？我文档里加了下，我来提交下
There was a problem hiding this comment.
There was a problem hiding this comment.
绝对路径是要从 /Users/xxx/repo/env, 还是 <rootDir>/env, 还是 /env 这样的
There was a problem hiding this comment.
绝对路径就是文件完整的路径...没有额外的设定，所以感觉不用特别说明
| build: { | ||
| desc: 'Clean, generate & upload result file', | ||
| handler() { | ||
| return clean().then(generate).then(upload) |
There was a problem hiding this comment.
fec-builder clean && fec-builder generate 就可以了
bin/fec-builder
Outdated
|
|
||
| const path = require('path') | ||
| const child_process = require('child_process') | ||
| let yargs = require('yargs') |
|
yarn 先不急，单独 PR 做吧。 现在基础 image 用的是 node:6.9.1，对应的 npm 版本是 3.10.8，而 npm 5.x 才支持 lock 吧我记得，所以要换的话还得换 image，或者在 build 时更新 npm，怕有坑哈哈 |
| 则代码中： | ||
|
|
||
| ```js | ||
| const apiUrl = API_PREFIX + 'test' |
There was a problem hiding this comment.
这样子的话，会不会太占全局变量了，起名字的时候负担比较大， 改成 ENV.API_PREFIX， 或者其他名字 ？
There was a problem hiding this comment.
我觉得这个能力可以放开来，用户可以自己配置成：
{
"ENV.API_PREFIX": "http://foobar.com/api",
"ENV.DEV": true
}这个样子的
There was a problem hiding this comment.
我们默认加一层 ENV 的话，一来会限制能力，如果需求就是某个非 ENV.* 的变量名，那就满足不了了（比如很多第三方库会默认读 process.env.NODE_ENV）；二来会多一层变量名的转换逻辑，不直观，也需要额外的文档说明
There was a problem hiding this comment.
现在的实现应该是可以写成下面这样的吧
{
"ENV": {
"API_PREFIX": "http://foobar.com/api",
"DEV": true
}
}
There was a problem hiding this comment.
按现在的实现，会把 ENV 替换成这个 object
There was a problem hiding this comment.
所以还是推荐写完整 ENV.API_PREFIX 这样的方式，webpack 倒也推荐这种方式，不过是针对 process.*，理由不同：https://webpack.js.org/plugins/define-plugin/#feature-flags
|
@doxiaodong 你看还有别的问题不 |
|
最好是拉下去代码你本地用试试，没有集成测试好慌… |
|
不要慌，不要慌，我空了看看 |
|
已测试 |
2 participants
