提高gitment的浏览器兼容性 #52

GeekaholicLin · 2017-07-21T16:40:45Z

所做的修改

配置文件以及源文件：增加babel对Promise以及Object.assign的转换，增强浏览器的兼容，可以兼容到IE10(但是样式可能会错乱)
npm script以及跨平台脚本(使用了cross-env)：方便npm run build后直接生成gitment.browser.js以及gitment.browser.min.js

GeekaholicLin · 2017-07-21T16:53:42Z

相关的issue : #50

可以使用IE10以及10以上打开页面 JavaScript模块化的一二事进行查看兼容性如何。
因为一些原因，还有一些浏览器尚未测试（FF番茄不了，移动设备有限）。

如果想在合并之前在自己的博客中使用，可以使用以下文件：（后者已压缩）

http://blog.geekaholic.cn/js/thirdParty/gitment.browser.js
http://blog.geekaholic.cn/js/thirdParty/gitment.browser.min.js

imsun · 2017-07-21T18:36:39Z

src/theme/default.js

+  container.appendChild(renderHeader(state, instance))
+  container.appendChild(renderComments(state, instance))
+  container.appendChild(renderEditor(state, instance))
+  container.appendChild(renderFooter(state, instance))


默认渲染函数之所以用 instance 调用渲染函数，是为了方便自定义主题。

比如一个人想自定义一下 footer，那他只要写一个 { renderFooter: () => { /* ... */} } 设为主题就好了，而不用将其他所有部分的逻辑都写一遍。自定义的 renderFooter 会覆盖 instance 上默认的 renderFooter 进行渲染。

imsun · 2017-07-21T18:47:01Z

package.json

-    "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"


没有理解为什么需要 postbuild。

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

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

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

imsun · 2017-07-21T18:48:54Z

webpack.config.js

+    devtool: 'source-map',
+    output: {
+      path: path.join(__dirname, 'dist'),
+      filename: (env&&env.production)?'gitment.browser.min.js':'gitment.browser.js',


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

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

GeekaholicLin

额，第一次用review功能，不大会使用，望见谅。还有一个被隐藏了，this.theme = Object.assign({},defaultTheme,options.theme);的改动是将所有的render method (包括default以及options)都统一挂载到this.theme下

GeekaholicLin · 2017-07-22T05:42:18Z

src/gitment.js

      oauth: {},
      perPage: 20,
      maxCommentHeight: 250,
    }, options)
-
+    this.theme = Object.assign({},defaultTheme,options.theme);


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

GeekaholicLin · 2017-07-22T05:46:17Z

src/gitment.js

-    renderers.forEach(renderer => extendRenderer(this, renderer))
+    const renderers = Object.keys(theme)
+    renderers.forEach(renderer => this[renderer]=this.theme[renderer])
+    extendRenderer(this,'render')


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

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

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

原来如此~引入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，但初步断定是在渲染的时候出错，所以目前需要解决的也是这个问题。

imsun · 2017-07-22T07:15:28Z

src/gitment.js

-    renderers.forEach(renderer => extendRenderer(this, renderer))
+    const renderers = Object.keys(theme)
+    renderers.forEach(renderer => this[renderer]=this.theme[renderer])
+    extendRenderer(this,'render')


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

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

imsun · 2017-07-22T07:43:21Z

package.json

-    "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"


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

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

imsun · 2017-07-22T07:49:26Z

webpack.dev.config.js

@@ -13,7 +13,7 @@ module.exports = {
    loaders: [
      {
        test: /\.js$/,
-        exclude: /^node_mocules/,
+        exclude: /^node_modules/,


我竟然还写过这种代码。震惊。

GeekaholicLin · 2017-07-22T11:22:06Z

src/theme/default.js

-  container.appendChild(instance.theme.renderEditor(state, instance))
-  container.appendChild(instance.theme.renderFooter(state, instance))
+  container.appendChild(instance.renderHeader(state, instance))
+  container.appendChild(instance.renderComments(state, instance))


找到IE的问题所在了。。确实是API的问题。我们简单地走一下流程。当调用gitment.render(container)的时候，实际上是调用instance.render(container)，而在instance.render(container)中获取了instance.theme[renderer]也就是this.theme.render(state, instance)，是渲染的总入口(囊括了Header等)，但是在this.theme.render(state, instance)中调用，比如默认的this.theme.render(state, instance)是调用以下：

container.appendChild(instance.renderHeader(state, instance)) container.appendChild(instance.renderComments(state, instance)) container.appendChild(instance.renderEditor(state, instance)) container.appendChild(instance.renderFooter(state, instance))

问题来了，instance.renderHeader(state, instance)的使用是错误的，签名对应不上，正确的参数列表应该是(container)，因为instance.renderHeader接收的是一个element或字符串。而实际上传进去的state和instance，那container是一个state对象，正确应该是使用instance.theme.renderxxxx(state, instance)（只渲染不操作，因为操作已经在container.appendChild有了呀），不过并无大影响，因为你的isString()已经帮你过滤了不正确的参数,粗略看了一下，不过是外加了一个div。所以本来改了API后又改了回来

但是对于IE不通过的原因是，toString不兼容。。所以加了一个空对象。

完善的如何了，最新的引用地址是哪个？

非官方的引用地址可以看上面的记录，等作者有空了并且review通过了应该就可以merge了。

http://blog.geekaholic.cn/js/thirdParty/gitment.browser.js
http://blog.geekaholic.cn/js/thirdParty/gitment.browser.min.js

好的，先用着你的...

Hi, @GeekaholicLin . 大佬，刚看到你博客了，有兴趣互相添加友链否？宇宙湾（https://yuzhouwan.com）

imsun/gitment#52

GeekaholicLin added 5 commits July 21, 2017 17:33

browser compatibility

bd261e7

fix config error && add npm script

aa7c305

fix browser compatibility for IE11/10 etc.

ffa970e

support uglifying js

9fdfef5

remove console.log

bdd45fe

imsun requested changes Jul 21, 2017

View reviewed changes

GeekaholicLin added 3 commits July 22, 2017 13:20

fix bugs using instance && do not change api as posible

1c68090

format

54af3cb

format

2f9a7ba

GeekaholicLin commented Jul 22, 2017

View reviewed changes

imsun requested changes Jul 22, 2017

View reviewed changes

imsun reviewed Jul 22, 2017

View reviewed changes

GeekaholicLin added 2 commits July 22, 2017 18:42

api break && webpack config and npm script optimize

f0e2a0b

fix toString.call not working in IE && recover origin api

50ff9dd

GeekaholicLin commented Jul 22, 2017

View reviewed changes

tiexo mentioned this pull request Sep 13, 2017

提高移动端兼容性 gitalk/gitalk#25

Closed

catusax added a commit to catusax/hexo-theme-apollo that referenced this pull request Oct 22, 2018

gitment.js转移到本地

53a4468

imsun/gitment#52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

提高gitment的浏览器兼容性 #52

提高gitment的浏览器兼容性 #52

GeekaholicLin commented Jul 21, 2017

GeekaholicLin commented Jul 21, 2017

imsun Jul 21, 2017

imsun Jul 21, 2017

GeekaholicLin Jul 22, 2017

imsun Jul 22, 2017

imsun Jul 21, 2017

GeekaholicLin left a comment

GeekaholicLin Jul 22, 2017

GeekaholicLin Jul 22, 2017

imsun Jul 22, 2017

GeekaholicLin Jul 22, 2017

imsun Jul 22, 2017

imsun Jul 22, 2017

imsun Jul 22, 2017

GeekaholicLin Jul 22, 2017

tiexo Jul 29, 2017

GeekaholicLin Jul 29, 2017

tiexo Jul 29, 2017

asdf2014 Jun 21, 2018 •

edited

Loading

提高gitment的浏览器兼容性 #52

Are you sure you want to change the base?

提高gitment的浏览器兼容性 #52

Conversation

GeekaholicLin commented Jul 21, 2017

所做的修改

GeekaholicLin commented Jul 21, 2017

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

GeekaholicLin 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

asdf2014 Jun 21, 2018 • edited Loading

Choose a reason for hiding this comment

asdf2014 Jun 21, 2018 •

edited

Loading