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.

Sign up for GitHub

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

Jump to bottom

fix: 修复h5环境下plugins-sass配置不生效的问题 #4474

Merged
merged 2 commits into from
Sep 23, 2019
Merged

fix: 修复h5环境下plugins-sass配置不生效的问题 #4474

merged 2 commits into from
Sep 23, 2019

Conversation

Liaozzzzzz
Copy link
Contributor

@Liaozzzzzz Liaozzzzzz commented Sep 17, 2019 •
edited
Loading

这个 PR 做了什么? (简要描述所做更改)
修复了h5环境下plugins-sass配置不生效的问题

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

  • 错误修复(Bugfix) issue id plugins.sass配置在H5环境无效 #4443
  • 新功能(Feature)
  • 代码重构(Refactor)
  • TypeScript 类型定义修改(Typings)
  • 文档修改(Docs)
  • 代码风格更新(Code style update)
  • 其他，请描述(Other, please describe):

这个 PR 满足以下需求:

  • 提交到 master 分支
  • Commit 信息遵循 Angular Style Commit Message Conventions
  • 所有测试用例已经通过
  • 代码遵循相关包中的 .eslintrc, .tslintrc, .stylelintrc 所规定的规范
  • 在本地测试可用，不会影响到其它功能

这个 PR 涉及以下平台:

  • 微信小程序
  • 支付宝小程序
  • 百度小程序
  • 头条小程序
  • QQ 轻应用
  • 快应用平台（QuickApp）
  • Web 平台（H5）
  • 移动端（React-Native）

其它需要 Reviewer 或社区知晓的内容：

@Littly Littly self-assigned this Sep 19, 2019
@Littly Littly self-requested a review September 19, 2019 03:01
Littly
Littly requested changes Sep 19, 2019
View reviewed changes
Copy link
Contributor

@Littly Littly left a comment

Choose a reason for hiding this comment

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

感谢支持~ 需要加强下鲁棒性。另外，这段逻辑更建议写在util/chain.ts文件中。

packages/taro-webpack-runner/src/index.ts Outdated
plugins,
sassLoaderOption: {
...sassLoaderOption,
data: sassLoaderOption.data ? `${sassLoaderOption.data}${bundledContent}` : bundledContent
Copy link
Contributor

Choose a reason for hiding this comment

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

未传入sassLoaderOption时会报错

Copy link
Contributor Author

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.

image
build taro-webpack-runner的时候报这个错, 但是却正确输出了dist目录文件, 不知道你那边有遇到过没 (build master分支的时候也出现, 之前不会)

@Liaozzzzzz Liaozzzzzz requested a review from Littly September 20, 2019 05:43
@luckyadam luckyadam merged commit 5237995 into NervJS:master Sep 23, 2019
@Liaozzzzzz Liaozzzzzz deleted the bugfix/sass-plugin-h5 branch September 23, 2019 13:55
@taro-bot taro-bot bot mentioned this pull request Sep 28, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Littly Littly Awaiting requested review from Littly

Assignees

@Littly Littly

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Liaozzzzzz @Littly @luckyadam