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.

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

fix(loader): support @charset removing and @import without url() #453

Merged
merged 2 commits into from Apr 11, 2022

Conversation

yanni4night
Copy link
Contributor

PR Details

  • 支持无url()的@import语句
  • 移除@charset语句
  • 更新了匹配url()的正则

Description

  1. 将无url()的@import语句转换为有url(),以供后续统一处理;
  2. @charset在<style>无意义,直接移除;
  3. url()可匹配带文件名带括号的资源

Related Issue

#446

Motivation and Context

  1. 支持了无url()的@import语句,兼容性更好
  2. 顺带移除了可能出现的@charset语句,减少控制台warning
  3. 优化了url()匹配模式,能兼容更多合理的资源名

How Has This Been Tested

repo中无相关test case,可手动在 https://github.com/yanni4night/garfish-issue-reproduction/tree/issue-446 验证。

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@zhoushaw zhoushaw requested a review from imtaotao April 7, 2022 14:41
@imtaotao
Copy link
Collaborator

imtaotao commented Apr 8, 2022

@import 的支持可以先不用,因为 import 进来的 css 文件里面可能还需要处理,里面的路径还需要修正

@codecov
Copy link

codecov bot commented Apr 8, 2022

Codecov Report

Merging #453 (7c309c6) into main (b742a2d) will increase coverage by 0.12%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #453      +/-   ##
==========================================
+ Coverage   78.70%   78.83%   +0.12%     
==========================================
  Files          84       84              
  Lines       10791    10795       +4     
  Branches     1502     1509       +7     
==========================================
+ Hits         8493     8510      +17     
+ Misses       2290     2277      -13     
  Partials        8        8              
Impacted Files Coverage Δ
packages/loader/src/managers/style.ts 65.67% <100.00%> (+24.68%) ⬆️
packages/browser-vm/src/modules/uiEvent.ts 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d3d69e...7c309c6. Read the comment docs.

@yanni4night
Copy link
Contributor Author

@import 的支持可以先不用,因为 import 进来的 css 文件里面可能还需要处理,里面的路径还需要修正

应该能应付绝大多数情况,e.g. 被import进来的box.css的路径被修正后,其里面的相对引用也是自动纠正的。

<!-- main.html -->
<div __garfishmockhtml__="">
   <div __garfishmockhead__="">
        <style type="text/css">
          @import url("http://localhost:9000/box.css");
       </style>
    </div>
</div>
/* box.css */
@import "./size.css";

.box {
  background: url(./bg.jpg);
}

@imtaotao
Copy link
Collaborator

@import 的支持可以先不用,因为 import 进来的 css 文件里面可能还需要处理,里面的路径还需要修正

应该能应付绝大多数情况,e.g. 被import进来的box.css的路径被修正后,其里面的相对引用也是自动纠正的。

<!-- main.html -->
<div __garfishmockhtml__="">
   <div __garfishmockhead__="">
        <style type="text/css">
          @import url("http://localhost:9000/box.css");
       </style>
    </div>
</div>
/* box.css */
@import "./size.css";

.box {
  background: url(./bg.jpg);
}

可以针对这些场景,补充一些测试用例

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

Successfully merging this pull request may close these issues.

None yet

2 participants