Skip to content
This repository has been archived by the owner on Mar 7, 2024. It is now read-only.

fix: rollup watch #173

Merged
merged 1 commit into from
Aug 30, 2019
Merged

fix: rollup watch #173

merged 1 commit into from
Aug 30, 2019

Conversation

Crayonn
Copy link
Contributor

@Crayonn Crayonn commented Aug 28, 2019

不好意思,更新晚了,现在extraFiles数组可以添加文件的配置,这部分的watch是要重新build的,把需要的监听文件放进去就可以了

@Darmody Darmody changed the title fix: rollup watch #148 fix: rollup watch Aug 28, 2019
@Darmody Darmody mentioned this pull request Aug 28, 2019
@Darmody
Copy link
Contributor

Darmody commented Aug 28, 2019

🆒

@Crayonn
Copy link
Contributor Author

Crayonn commented Aug 28, 2019

🆒

有什么问题,呼叫我就可以了😀

@yesmeck
Copy link
Member

yesmeck commented Aug 28, 2019

Screen Shot 2019-08-28 at 2 55 46 PM

奇怪,我本地报这个错,已经 lerna bootstrap 过了。

@Darmody
Copy link
Contributor

Darmody commented Aug 28, 2019

Screen Shot 2019-08-28 at 2 55 46 PM

奇怪,我本地报这个错,已经 lerna bootstrap 过了。

之前我跑了一下也是这样

@Crayonn
Copy link
Contributor Author

Crayonn commented Aug 28, 2019

这是是build以后chokidar只存在remax-cli包里边,但是rollup 取得包是当前这个node_modules,地址:https://github.com/rollup/rollup/blob/cc5fd63d9a612d61499c3b7e8ee97957422856a8/src/watch/chokidar.ts#L7
那个包是用来轮训更改的file文件的,删除文件夹后也会rebuild

@Darmody
Copy link
Contributor

Darmody commented Aug 28, 2019

这是是build以后chokidar只存在remax-cli包里边,但是rollup 取得包是当前这个node_modules,地址:https://github.com/rollup/rollup/blob/cc5fd63d9a612d61499c3b7e8ee97957422856a8/src/watch/chokidar.ts#L7
那个包是用来轮训更改的file文件的,删除文件夹后也会rebuild

哦,那可以把 chokidar 取出来

https://github.com/remaxjs/remax/blob/master/packages/remax-cli/src/build/rollupConfig.ts#L140

@Crayonn
Copy link
Contributor Author

Crayonn commented Aug 28, 2019

这是是build以后chokidar只存在remax-cli包里边,但是rollup 取得包是当前这个node_modules,地址:https://github.com/rollup/rollup/blob/cc5fd63d9a612d61499c3b7e8ee97957422856a8/src/watch/chokidar.ts#L7
那个包是用来轮训更改的file文件的,删除文件夹后也会rebuild

哦,那可以把 chokidar 取出来

https://github.com/remaxjs/remax/blob/master/packages/remax-cli/src/build/rollupConfig.ts#L140

dedupe 这个是如果root有的话就默认加载root下边的node_module包,而不用直接加载自己node_module的包。而且watch实在rullop build之前做的,所以得直接将这个包弄出来,你有什么其他的解决方案吗?

@yesmeck
Copy link
Member

yesmeck commented Aug 28, 2019

这个 debupe 是编译小程序的时候用的,不应该影响 remax-cli 的运行才对。

@Darmody
Copy link
Contributor

Darmody commented Aug 28, 2019

哦是的

@Crayonn
Copy link
Contributor Author

Crayonn commented Aug 29, 2019

rollup/rollup#2988

看样子后边会更新这个问题,现在是等rollup更新还是在加载的时候自动添加chokidor这个包吗?目前没有想到其他的好的方式了

@Crayonn Crayonn closed this Aug 29, 2019
@Crayonn Crayonn reopened this Aug 29, 2019
@Darmody
Copy link
Contributor

Darmody commented Aug 29, 2019

在脚手架里把他加到开发依赖?

@Crayonn
Copy link
Contributor Author

Crayonn commented Aug 29, 2019

在脚手架里把他加到开发依赖?

现在rollup取chokidar包的时候是取的root node_modules下边的包,remax cli 是下载自己的node_modules下边,好像配置不到root node_modules下边

@Darmody
Copy link
Contributor

Darmody commented Aug 29, 2019

在脚手架里把他加到开发依赖?

现在rollup取chokidar包的时候是取的root node_modules下边的包,remax cli 是下载自己的node_modules下边,好像配置不到root node_modules下边

嗯,所以我意思在脚手架里让开发者安装 chokidar

@yesmeck
Copy link
Member

yesmeck commented Aug 29, 2019

是版本不匹配吗?

@Darmody
Copy link
Contributor

Darmody commented Aug 29, 2019

是版本不匹配吗?

https://github.com/rollup/rollup/blob/cc5fd63d9a612d61499c3b7e8ee97957422856a8/src/watch/chokidar.ts#L7

屏幕快照 2019-08-29 15 44 20

是 rollup 取 chokidar 的问题

@Crayonn
Copy link
Contributor Author

Crayonn commented Aug 29, 2019

是版本不匹配吗?
rollup/rollup#2988 (comment)
是这个问题,这个问题说是要跟update chokidar一块更新,我看着意思是。

@Darmody
Copy link
Contributor

Darmody commented Aug 29, 2019

我试了一下,删除掉 entry 会报错

屏幕快照 2019-08-29 16 31 55

@Darmody
Copy link
Contributor

Darmody commented Aug 29, 2019

监听 native 文件 OK。

不过 watch 每次好像是全部重新 build 一遍?

@Darmody
Copy link
Contributor

Darmody commented Aug 29, 2019

👍 效果不错。

目前我们需要额外监听的文件

屏幕快照 2019-08-29 16 51 56

目前的问题:
chokidar 安装问题,加入到脚手架中?
额外监听的文件变化,会全部 build 一遍,如果能优化一下更好。
还有一个问题就是上面提到的删掉 page 会报错。

@Crayonn
Copy link
Contributor Author

Crayonn commented Aug 29, 2019

报错那个是我做了验证,因为不提示感觉如果是路径问题排查错误不好找,这个验证可以去掉

刚才你提到的说重新build说是因为他不属于import一部分,所以无法做到只编译部分的。像native部分是全部的依赖,不得不重新rebuild。不知道我的想法有没有问题。

@Darmody
Copy link
Contributor

Darmody commented Aug 29, 2019

报错那个是我做了验证,因为不提示感觉如果是路径问题排查错误不好找,这个验证可以去掉

刚才你提到的说重新build说是因为他不属于import一部分,所以无法做到只编译部分的。像native部分是全部的依赖,不得不重新rebuild。不知道我的想法有没有问题。

哦,很合理。 所以删掉 entry 只是提示,打包是成功的?

@Crayonn
Copy link
Contributor Author

Crayonn commented Aug 29, 2019

报错那个是我做了验证,因为不提示感觉如果是路径问题排查错误不好找,这个验证可以去掉
刚才你提到的说重新build说是因为他不属于import一部分,所以无法做到只编译部分的。像native部分是全部的依赖,不得不重新rebuild。不知道我的想法有没有问题。

哦,很合理。 所以删掉 entry 只是提示,打包是成功的?

会断掉用的throw的。如果只是提示的话可以改一下,我觉得build如果没找到依赖应该算是错误,其实都合理。

@Darmody
Copy link
Contributor

Darmody commented Aug 29, 2019

这个相当于是把entry删掉了,我们可以不报错,至于没有对应的页面路径,小程序会有报错提示

@Crayonn
Copy link
Contributor Author

Crayonn commented Aug 29, 2019

这个相当于是把entry删掉了,我们可以不报错,至于没有对应的页面路径,小程序会有报错提示

那就改成 warning的那种方式,只是提醒就可以吗?
那个地方是我的想法,哈哈,可能提示的有点过了

@Darmody
Copy link
Contributor

Darmody commented Aug 29, 2019

这个相当于是把entry删掉了,我们可以不报错,至于没有对应的页面路径,小程序会有报错提示

那就改成 warning的那种方式,只是提醒就可以吗?

不用 warning,console.log 就行

@Crayonn
Copy link
Contributor Author

Crayonn commented Aug 29, 2019

这个相当于是把entry删掉了,我们可以不报错,至于没有对应的页面路径,小程序会有报错提示

那就改成 warning的那种方式,只是提醒就可以吗?

不用 warning,console.log 就行
好的

@Darmody
Copy link
Contributor

Darmody commented Aug 30, 2019

@Crayonn 你现在想怎么做?

@Crayonn
Copy link
Contributor Author

Crayonn commented Aug 30, 2019

@Darmody 我想了一下,可以按照yesmeck说的做,如果没有就不使用chokidar,我们可以提示一下告诉如果用chikodar体验会更好,这样可以不。

@Darmody
Copy link
Contributor

Darmody commented Aug 30, 2019

@Darmody 我想了一下,可以按照yesmeck说的做,如果没有就不使用chokidar,我们可以提示一下告诉如果用chikodar体验会更好,这样可以不。

OK

@Crayonn
Copy link
Contributor Author

Crayonn commented Aug 30, 2019

@Darmody 好了,你看看这样改可以吗?😆

} catch (e) {
console.log(e);
console.log(
'\nIf you install `chokidar`, you will be a better development experience! 😎'
Copy link
Member

@yesmeck yesmeck Aug 30, 2019

Choose a reason for hiding this comment

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

就写中文吧,开发小程序的基本上就是中文开发者了。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好👌

require(path.resolve(process.cwd(), './node_modules/chokidar'));
return true;
} catch (e) {
console.log('\n 如果下载 `chokidar`,会有更好的开发体验~~ 😎');
Copy link
Member

Choose a reason for hiding this comment

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

安装 `chokidar` 获得更好的开发体验~~😎

> npm install chokidar --save 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的

@Darmody
Copy link
Contributor

Darmody commented Aug 30, 2019

🎉🎉🎉

LGTM

@Darmody
Copy link
Contributor

Darmody commented Aug 30, 2019

@yesmeck OK 吗

@yesmeck
Copy link
Member

yesmeck commented Aug 30, 2019

好像新增页面不行,新增的页面不会被编译。

@Darmody
Copy link
Contributor

Darmody commented Aug 30, 2019

你有修改pages吗?

@Crayonn
Copy link
Contributor Author

Crayonn commented Aug 30, 2019

好像新增页面不行,新增的页面不会被编译。

需要添加到config文件中就可以了

@yesmeck
Copy link
Member

yesmeck commented Aug 30, 2019

重新 watch 的时候得重新获取 rollupConfig 才行,不然里面的 entries 还是旧的

@yesmeck
Copy link
Member

yesmeck commented Aug 30, 2019

Screen Shot 2019-08-30 at 11 34 35 PM

好像有内存泄漏

@yesmeck
Copy link
Member

yesmeck commented Aug 30, 2019

删掉 app.config.js 里的一个页面就会 OOM。

不对,好像是多 runWatch 几次会 OOM。

@yesmeck
Copy link
Member

yesmeck commented Aug 30, 2019

在 app.config.js 里新增、删除 page,多来几次就 OOM 了。

@Crayonn Crayonn force-pushed the watch-rollup branch 2 times, most recently from 436de61 to 9908c4f Compare August 30, 2019 22:28
@Crayonn
Copy link
Contributor Author

Crayonn commented Aug 30, 2019

在 app.config.js 里新增、删除 page,多来几次就 OOM 了。

监听的时候没有释放掉之前的watch,昨天晚上我用reload连续跑了一次
run
没有crash

entries 也更新了现在

@Darmody
Copy link
Contributor

Darmody commented Aug 30, 2019

我试了下没有泄露了

就是额外监听的文件需要重新 build 体验上有点差,后面再想想办法吧

@Darmody Darmody merged commit f2f15ba into remaxjs:master Aug 30, 2019
@Darmody Darmody mentioned this pull request Aug 30, 2019
@Crayonn
Copy link
Contributor Author

Crayonn commented Aug 31, 2019

我试了下没有泄露了

就是额外监听的文件需要重新 build 体验上有点差,后面再想想办法吧

我最近我也在想有什么更好的方式处理这个问题

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants