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

feat: support css @import hmr #281

Merged
merged 3 commits into from
May 29, 2020
Merged

feat: support css @import hmr #281

merged 3 commits into from
May 29, 2020

Conversation

underfin
Copy link
Member

close #9

test/test.js Outdated
// await updateFile('css-@import/testCssImportBoundary.scss', (content) =>
// content.replace('red', 'rgb(0, 0, 0)')
// )
// await expectByPolling(() => getComputedColor(el), 'rgb(0, 0, 0)')
Copy link
Member Author

@underfin underfin May 28, 2020

Choose a reason for hiding this comment

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

This test is ok when run dev and test in broswer.But it fail in test.I don't know why.So I'm need you take a look. Thanks!
This is important for support css module with preprocessors.But I meet this error.And I almost implement it in this pr....I will work that after.

return css
}
return await postcss().use(cssImport()).process(css, { from: filePath })
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This maybe can be combinate with compileStyleAsync use postcss-easy-import plugin(I meet some error with this). And it need compiler-sfc export postcss messages.
So I only implement this...

@underfin underfin changed the title feat: support css @import feat: support css @import hmr May 29, 2020
@yyx990803
Copy link
Member

yyx990803 commented May 29, 2020

@vue/compiler-sfc actually returns the raw result from PostCSS, so we can merge the postcss-import plugin into compileSFCStyle call, which is more efficient.

Another thing is your test for @import with scss only worked because the scss code you were testing happened to be valid CSS as well, so PostCSS was able to parse it. However if the code contains non CSS syntax, it will break. So I removed the tests and now it only tests @import from plain CSS files.

To properly support @import in pre-processors we need to use the import-tracking APIs from those pre-processors themselves, e.g. for sass it would be the importer option. However, since that is handled by @vue/compiler-sfc, we will have to implement that kind of logic in @vue/compiler-sfc instead. Maybe we can add a field to the SFC compile result called dependencies (which will include either the deps from the PostCSS result or other pre-processors).

@yyx990803 yyx990803 merged commit 9bc3fbd into vitejs:master May 29, 2020
@underfin
Copy link
Member Author

@yyx990803. This plugin look like is only parse@import (it not parser scss syntax) and export all @import to one file.I test it again and the reslut like the below .

// index.js
// dependencies
var fs = require("fs")
var postcss = require("postcss")
var atImport = require("postcss-import")

// css to be processed
var css = fs.readFileSync("./a.css", "utf8")

// process css
postcss()
    .use(atImport())
    .process(css, {
      // `from` option is needed here
      from: "./a.css?a=1"
    })
    .then(function (result) {
      var output = result.css
      console.log(result)
      console.log(output)
    })
// a.css
@import "c.scss";
// c.scss
$red: red
body {
  background: $red;
}
$content: "First content";
$content: "Second content?" !default;
$new_content: "First time reference" !default;

#main {
  content: $content;
  new-content: $new_content;
  .a {
    color: red;
  }
}

p {
  color: #010203 * 2;
}

a {
  &:hover { text-decoration: underline; }
  body.firefox & { font-weight: normal; }
}
@mixin large-text {
  color: #ff0000;
}
.page-title {
  @include large-text;
}

p {
  @if 1 + 1 == 2 { border: 1px solid; }
  @if 5 < 3 { border: 2px dotted; }
  @if null  { border: 3px double; }
}

This will output this below.

Result {
  processor: Processor { version: '6.0.23', plugins: [ [Function] ] },
  messages: [
    {
      type: 'dependency',
      plugin: 'postcss-import',
      file: '/Users/likui/code/demo/test/c.scss',
      parent: '/Users/likui/code/demo/test/a.css?a=1'
    }
  ],
  root: Root {
    raws: { semicolon: true, after: '\n\n' },
    type: 'root',
    nodes: [
      [Rule],        [Declaration],
      [Declaration], [Declaration],
      [Rule],        [Rule],
      [Rule],        [AtRule],
      [Rule],        [Rule]
    ],
    source: { input: [Input], start: [Object] },
    lastEach: 2,
    indexes: {}
  },
  opts: { from: './a.css?a=1' },
  css: '$red: red\n' +
    'body {\n' +
    '  background: $red;\n' +
    '}\n' +
    '$content: "First content";\n' +
    '$content: "Second content?" !default;\n' +
    '$new_content: "First time reference" !default;\n' +
    '#main {\n' +
    '  content: $content;\n' +
    '  new-content: $new_content;\n' +
    '  .a {\n' +
    '    color: red;\n' +
    '  }\n' +
    '}\n' +
    'p {\n' +
    '  color: #010203 * 2;\n' +
    '}\n' +
    'a {\n' +
    '  &:hover { text-decoration: underline; }\n' +
    '  body.firefox & { font-weight: normal; }\n' +
    '}\n' +
    '@mixin large-text {\n' +
    '  color: #ff0000;\n' +
    '}\n' +
    '.page-title {\n' +
    '  @include large-text;\n' +
    '}\n' +
    'p {\n' +
    '  @if 1 + 1 == 2 { border: 1px solid; }\n' +
    '  @if 5 < 3 { border: 2px dotted; }\n' +
    '  @if null  { border: 3px double; }\n' +
    '}\n' +
    '\n',
  map: undefined,
  lastPlugin: [Function] {
    postcssPlugin: 'postcss-import',
    postcssVersion: '7.0.31'
  }
}
$red: red
body {
  background: $red;
}
$content: "First content";
$content: "Second content?" !default;
$new_content: "First time reference" !default;
#main {
  content: $content;
  new-content: $new_content;
  .a {
    color: red;
  }
}
p {
  color: #010203 * 2;
}
a {
  &:hover { text-decoration: underline; }
  body.firefox & { font-weight: normal; }
}
@mixin large-text {
  color: #ff0000;
}
.page-title {
  @include large-text;
}
p {
  @if 1 + 1 == 2 { border: 1px solid; }
  @if 5 < 3 { border: 2px dotted; }
  @if null  { border: 3px double; }
}

@underfin
Copy link
Member Author

underfin commented May 30, 2020

I check the implement of postcss-import. See https://github.com/postcss/postcss-import/blob/master/lib/parse-statements.js#L24.
This plugin really is only parse@import (it not parser scss syntax) and export all @import to one file.

So we maybe can't merge postcss-import into compileStyleAsync.Just leave out it before compileStyleAsync.And I will implement parser import syntax without postcss-import and approach the aim of improve performance.

@underfin
Copy link
Member Author

=。=you are right.I get syntax error.

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.

HMR when using imported sass files
2 participants