Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
3 changes: 2 additions & 1 deletion .babelrc
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{
"presets": ["es2015"]
"presets": ["es2015"],
"plugins": ["transform-runtime"]
}
8 changes: 6 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,22 @@
},
"homepage": "https://github.com/imsun/gitment",
"scripts": {
"build": "babel src --out-dir dist --ignore test.js --source-maps & NODE_ENV=production webpack --config webpack.config.js --progress --profile --colors",
"dev": "webpack-dev-server --config webpack.dev.config.js --host 0.0.0.0 --progress --profile --colors"
"build": "babel src --out-dir dist --ignore test.js --source-maps & cross-env NODE_ENV=production webpack --config webpack.config.js --progress --profile --colors",
"dev": "webpack-dev-server --config webpack.dev.config.js --host 0.0.0.0 --progress --profile --colors",
"postbuild": "babel src --out-dir dist --ignore test.js --source-maps & webpack --config webpack.config.js -p --env.production"
Copy link
Owner

Choose a reason for hiding this comment

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

没有理解为什么需要 postbuild。

Copy link
Author

Choose a reason for hiding this comment

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

这样的话可以使用npm run build完成后自动生成压缩版本的gitment,当然可以直接生成压缩版本,但是为了不改动太大,所以就保留了你原有的脚本,在原有基础上添加

Copy link
Owner

Choose a reason for hiding this comment

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

这个和 postbuild 的语义不大相符。而且 postbuild 是接着 build 执行的, 前半段命令在 build 时已经执行过了,没有必要再执行一遍。另外我也不建议通过 env.production 做标识来判断是否需要压缩,因为已经有 NODE_ENV 了,都是 production 语义上却有分歧。如果加压缩应该在 build 脚本里同时 build 出两个文件。

至于展开来说,我当时没加压缩主要是因为现在的 server 都开了 gzip,压不压缩意义不大。不过加了我也没有意见。

},
"devDependencies": {
"babel-cli": "^6.24.0",
"babel-core": "^6.24.0",
"babel-loader": "^6.4.1",
"babel-plugin-transform-runtime": "^6.23.0",
"babel-preset-es2015": "^6.24.0",
"cross-env": "^5.0.1",
"webpack": "^2.3.2",
"webpack-dev-server": "^2.4.2"
},
"dependencies": {
"babel-runtime": "^6.23.0",
"mobx": "^3.1.7"
},
"license": "MIT"
Expand Down
15 changes: 5 additions & 10 deletions src/gitment.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ const scope = 'public_repo'
function extendRenderer(instance, renderer) {
instance[renderer] = (container) => {
const targetContainer = getTargetContainer(container)
const render = instance.theme[renderer] || instance.defaultTheme[renderer]

const render = instance.theme[renderer]
autorun(() => {
const e = render(instance.state, instance)
if (targetContainer.firstChild) {
Expand Down Expand Up @@ -46,20 +45,17 @@ class Gitment {

constructor(options = {}) {
this.defaultTheme = defaultTheme
this.useTheme(defaultTheme)

Object.assign(this, {
id: window.location.href,
title: window.document.title,
link: window.location.href,
desc: '',
labels: [],
theme: defaultTheme,
oauth: {},
perPage: 20,
maxCommentHeight: 250,
}, options)

this.theme = Object.assign({},defaultTheme,options.theme);
Copy link
Author

Choose a reason for hiding this comment

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

将所有的render method (包括default以及options)都统一挂载到this.theme

this.useTheme(this.theme)

const user = {}
Expand Down Expand Up @@ -127,10 +123,9 @@ class Gitment {
}

useTheme(theme = {}) {
this.theme = theme

const renderers = Object.keys(this.theme)
renderers.forEach(renderer => extendRenderer(this, renderer))
const renderers = Object.keys(theme)
renderers.forEach(renderer => this[renderer]=this.theme[renderer])
extendRenderer(this,'render')
Copy link
Author

Choose a reason for hiding this comment

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

貌似你原本的逻辑是将所有的render method都挂载到instance下,然后供render函数调用。但是问题在于extendRenderer()函数对各个render method进行包装,改变了render method的参数,由(state,instance)变为container。而正确的做法应该是只对render包装即可,其余的render method简单地挂到instance下,供render函数调用

Copy link
Owner

Choose a reason for hiding this comment

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

extendRenderer 是将 theme 里的 renderer 暴露为 API,所以需要接收一个 container 用于渲染。而 theme 里的 renderer 不需要关心这些,只要接收 state 返回一个 DOM element 就可以了。

这两个的区别在于,instance 上的 renderer 是用户传参调用的,theme 里的 renderer 是被内部调用的,所以接受的参数不一样。比如用户只想渲染一个评论列表时就可以 gitment.renderComments(document.body)

Copy link
Author

Choose a reason for hiding this comment

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

原来如此~引入babel-plugin-transform-runtime后语法上的兼容性基本没有问题,但是在IE10下Mobx会报错。

[mobx] Encountered an uncaught exception that was thrown by a reaction or observer component, in: 'Reaction[Autorun@2] TypeError: 调用的对象无效

没有接触过Mobx,但初步断定是在渲染的时候出错,所以目前需要解决的也是这个问题。

}

update() {
Expand Down
1 change: 1 addition & 0 deletions src/theme/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ function renderHeader({ meta, user, reactions }, instance) {
likeButton.classList.remove('liked')
likeButton.onclick = () => instance.like()
}

container.appendChild(likeButton)

const commentsCount = document.createElement('span')
Expand Down
42 changes: 22 additions & 20 deletions webpack.config.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,24 @@
const path = require('path')

module.exports = {
context: path.join(__dirname, 'src'),
entry: './gitment.js',
devtool: 'source-map',
output: {
path: path.join(__dirname, 'dist'),
filename: 'gitment.browser.js',
libraryTarget: 'var',
library: 'Gitment',
},
module: {
loaders: [
{
test: /\.js$/,
exclude: /^node_mocules/,
loaders: ['babel-loader'],
},
],
},
}
module.exports = function (env) {
return {
context: path.join(__dirname, 'src'),
entry: './gitment.js',
devtool: 'source-map',
output: {
path: path.join(__dirname, 'dist'),
filename: (env&&env.production)?'gitment.browser.min.js':'gitment.browser.js',
Copy link
Owner

Choose a reason for hiding this comment

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

还没加 lint。所以请手动在 operator 间补下空格。

(env && env.production) ? 'gitment.browser.min.js' : 'gitment.browser.js'

libraryTarget: 'var',
library: 'Gitment',
},
module: {
loaders: [
{
test: /\.js$/,
exclude: /node_modules/,
loaders: ['babel-loader'],
},
],
},
}
};