Problem/Motivation

While implementing the new design system, the team raised some interest in using some of the new CSS features (mainly CSS custom properties or CSS variables). Being able to leverage these new features would lead to less overhead in the maintenance.

For example, CSS custom properties are currently supported by latest browser versions, while some older browsers are lacking support (browser support matrix). However, with the usage of CSS preprocessor, we could already reliably use CSS custom properties.

Proposed resolution

Use PostCSS to compile a version of our CSS that is supported by the browsers Drupal currently support. PostCSS is used by industry leaders including Wikipedia, Twitter, Alibaba, and JetBrains. PostCSS has over 20 000 stars in Github and is being used by almost 2 million projects according to Github.

Plugins we are currently using:

  • Autoprefixer is a plugin to parse CSS and add vendor prefixes to CSS rules using values from Can I Use. It is recommended by Google and used in Twitter and Alibaba.
  • PostCSS Calc reduces calc() references whenever it's possible. This can be particularly useful with the PostCSS Custom Properties plugin.
  • PostCSS Custom Properties lets us use Custom Properties in CSS, following the CSS Custom Properties specification.
  • PostCSS Inline plugin to transform @import rules by inlining content. This enables importing variables from a central file.

Dependency evaluation:

  • Maintainership of the package: PostCSS project itself has multiple maintainers but it seems that only one of them is active in the core. Full list of maintainers. PostCSS repositories are active and PR's seem to receive feedback very quickly. PostCSS is used by at least 2.7 million other packages. PostCSS is also used by some large organizations including Wikipedia, Twitter, Alibaba, and JetBrains.
  • Security policies of the package: Security issues can be reported privately using Tidelift. This library is only used in by core developers in development environments.
  • Expected release and support cycles: Looking at the history of the library, there has been a new major release every 2 years. Only single major release is supported at the time.
  • Code quality: APIs are well documented and the library has sufficient test coverage.
  • Other dependencies it would add, if any: list of additional dependencies

Development workflow Q&A

How would this affect the developer workflow?

Core developers will have to run yarn run build:css while making changes to CSS. At first, this will only affect CSS in Claro but eventually, given that the tooling is there, this is something that could be expanded outside of Claro.

So now I need to run not only yarn run watch:js but also yarn run watch:css, doesn't this harm DX?

If you're working on a theme using PostCSS and using ES6 JS, then you can just do yarn run watch to do both yarn run watch:js and yarn run watch:css at the same time 👍

Would it affect only core developers, or would there be additional required steps for contrib or sites as well?

This tooling is designed to be specific to core and doesn't affect contrib in any way.

Would we handle it the way we do ES5 transpilation, with patch contributors responsible for including both the source and the compiled version, and committers' toolchains verifying it on commit?

Yes.

What's the plan for the dependency if Claro is not added to core?

This is indeed added mainly Claro in mind, but this isn't specific to Claro in any way. For example, the new frontend theme (#3064880: Create new default front-end theme) is planning to use PostCSS as well.

Remaining tasks

None.

Release notes snippet

N/A

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lauriii created an issue. See original summary.

chicagomom’s picture

Using PostCSS could provide an additional opportunity: using the PostCSS Style Guide plugin (Github) to generate style guides using markdown. There have been several discussions concerning the need for a live style guide for the admin theme (and perhaps all themes) using a variety of proposed methods. For more background:

AaronMcHale’s picture

Re #2: @2chicagomom big +1 to that, I’m always a fan of using automation to improve processes and a living automated style guide would be a great way to reduce manual work, as well as guarantee it’s always up to date.

mherchel’s picture

+1 to this. We hope to use PostCSS on a new Drupal core front-end theme, too.

catch’s picture

How does this impact people who need to make a one-off change to core CSS? Do you need to build the CSS and add the changes to the patch?

My main experience of CSS post-processors, because I hardly ever do front-end development, is having to navigate out-of-date instructions on what to run to build the CSS, running into mis-matching versions etc. - how susceptible is postcss to that kind of problem?

lauriii’s picture

How does this impact people who need to make a one-off change to core CSS? Do you need to build the CSS and add the changes to the patch?

The impact would be comparable to the process we currently have for JavaScript. People would have to run a command while working on a patch. I don't think we have another way to handle this since we don't have a build process where we could add this easily.

My main experience of CSS post-processors, because I hardly ever do front-end development, is having to navigate out-of-date instructions on what to run to build the CSS, running into mis-matching versions etc. - how susceptible is postcss to that kind of problem?

I've run into similar issues but it's only been when the stack has been built on Ruby which was for a long while the most commonly used language for compiling frontend assets. However, we are using JavaScript so the tooling is similar to what we already use for our core JavaScript, including the package manager. There shouldn't be issues with out-of-date instructions since there are just two commands people generally have to run, an they're both controlled by us (again similar to what we have for JavaScript). This also means that even if our external dependencies change their commands, that wouldn't affect our users.

gueguerreiro’s picture

Issue summary: View changes

Updating the issue summary to fix malformed links.

xjm’s picture

Issue summary: View changes

Let's put the dependency evaluation in the IS too. Some of the points are already covered by the proposed resolution, but not all of them.

alexpott’s picture

I'm +1 to using the same modern build tools that everyone else is using to build frontends. Especially as the selected tool is using the same tools to manage dependencies as our existing JS build process. Also as this is only for development and does not affect Drupal's runtime code there are less security concerns than we have with adding new PHP dependencies.

I agree with @catch that often the tooling and build instructions can cause problems however I'm hopefully that as we're going to build on our existing devtools and commit the final CSS files (like we do for JS) we can mitigate this.

andrewmacpherson’s picture

I have no strong opinion on PostCSS itself (or any other preprocessor), but I'm very interested in the automatically updated living styleguide mentioned in #2.

@chicagomom - is the generated styleguide a real HTML document using the resulting stylesheet? As opposed to generated screenshots from a headless browser, say.

If the styleguide is real HTML then it gets a big +1000 from me. Such living styleguides are very handy for manual accessibility review and regression testing; we can point a checker like aXe at them, or load them in a browser to for manual testing of focus styles, WCAG resize/reflow criteria, etc.

droplet’s picture

#10

Not that smart but increasing work on both the patcher or reviewer. The styleguide is a generator for human annotations code and required a modified version to adopt the PHP code (via Drupal FORM API or theme functions ..etc.) to generate more recent HTML code.

There's a very common case in Drupal. The extra CSS files & CSS classes loaded from JS. And multiple CSS files from the CORE system, modules, stable theme until your actual theme. This doesn't end... Sometimes, you were using a child theme. You need a combination to get the final style.

The styleguide could be a quick demo for the themer to take a glance at different element components. e.g.: What's the shape and color for a warning button? But not for fully accessibility review or any patch reviewers.

With the limitations, to build a standard module to loop over all Drupal FORM API & Theme components in a single page is more efficiently. A system like storybook is advanced:
https://github.com/storybookjs/storybook

one small step for Claro, one giant leap for Drupal CORE
#2658650: [META] Optimize Frontend Workflow in Core Development

andrewmacpherson’s picture

#11: that doesn't answer my question. Does the generated styleguide have screenshots of the components, or HTML?

Are there any examples of styleguides I can try, generated with the plugin mentioned in #2? The GitHub README doesn't say so.

I've don't think I've ever heard the term "warning button" before. I'm not sure what one of those is.

lauriii’s picture

#12: the styleguide will have HTML and CSS rather than screenshots. However, I think we should review details related to that in a follow-up. There's strong arguments in favor of using PostCSS even if we didn't use it for generating styleguides.

alexpott’s picture

@catch and I are fine with this from a framework manager point of view. As per #9 I've outlined my reasons for why,

I think now we need to work on dependency evaluations as per #8 in order to satisfy the release manager review requirement.

lauriii’s picture

Issue summary: View changes
lauriii’s picture

Issue summary: View changes
lauriii’s picture

Issue summary: View changes
xjm’s picture

Issue tags: +Needs change record

A few questions:

  1. How would this affect the developer workflow?
  2. Would it affect only core developers, or would there be additional required steps for contrib or sites as well?
  3. Would we handle it the way we do ES5 transpilation, with patch contributors responsible for including both the source and the compiled version, and committers' toolchains verifying it on commit?
  4. What's the plan for the dependency if Claro is not added to core?

This would need a CR, and specific instructions for how it'll get used in our process it would be good as well.

Thanks everyone!

xjm’s picture

Also, if we're going to add new things to the committer dev toolchain, let's please do that well in advance of alpha and have complete instructions/roadmap/etc.

I could see committing Claro as alpha with just the compiled CSS, and using PostCSS as internal development tool until we're ready to go beta. Or we could add both as alpha, with them needing an rm -rf from 8.8.x. Just please don't change the committer workflow or toolchain during October.

xjm’s picture

Oh, we also need to evaluate the dependency tree, not just the dependency. That's a whole lot of dev dependencies. (I mean, it's par for the course in the JS world, but not something that's in Drupal core process yet outside the JS initiative.) So it's an important step that we should discuss and think about.

Would we be using the tool as a dev build or a prod build in our workflows? If it's a prod build, cool, just evaluate those other three dependencies. But if we get all those dev dependencies and any one of them can break our development build... well, that's us entering the scary JS zillion-dependency world. So we should also talk about what the consequences would be if we had a leftpad incident -- or malicious code -- in one of the dependencies.

xjm’s picture

Or, framed another way, let's document a sample yarn.lock of the build we'd use, and list the dependencies there, and document/evaluate them at least at a high level.

xjm’s picture

Oops, I realized one of my questions (about the workflow being like ES5 transpilation or not) was already answered by @lauriii in #6. Sorry! This goes back to the need for the CR and IS update. :)

lauriii’s picture

Status: Active » Needs review
FileSize
18.98 KB

Here's an attempt on a proposed workflow. This is very close to what we already have in place for ES6. In fact, I copied the architecture of their compilation process directly into this. This compiles *.pcss.css files into .css files and inserts a header.

These are the new dev dependencies added for Drupal Core as part of this (can be seen in the yarn.lock file:

  • color-name
  • css-unit-converter
  • cssesc
  • ip-regex
  • is-number
  • is-url-superb
  • postcss-calc
  • postcss-custom-properties
  • postcss-header
  • postcss-selector-parser
  • postcss
  • supports-color
  • tlds
  • url-regex
lauriii’s picture

Cleaned up some documentation and moved packages accidentally added as production dependencies to dev dependencies.

xjm’s picture

The patch is a huge help; thank you.

+++ b/core/package.json
@@ -84,5 +88,18 @@
+  "dependencies": {
+    "autoprefixer": "^9.6.1",
+    "postcss-calc": "^7.0.1",
+    "postcss-custom-properties": "^9.0.2"

So, if I'm reading right, these are the first non-dev dependencies in our core package.json. The rest, eslint and friends, are devDependencies. Is there a reason for the difference, and does it impact anything?

Also, just to confirm: While they're listed as prod dependencies, that doesn't (?) mean prod for sites with their own JS dependency management. These will only end up on their servers if they go into the core dir and <del>npm</del> yarn install, which they have no reason to do unless they're specifically working on core. Correct?

(One of the things we're always trying to evaluate is risk and exposure. Every dependency is a security risk, and so it's important to know how many laptops and servers might intentionally or unintentionally end up with new things installed.

Thanks!

xjm’s picture

#24 answers most of #25. Brain waves in synch!

xjm’s picture

Issue tags: +Needs followup

Okay, great, so it's actually adding very little besides itself and other things we use for like eslint and stylelint and the transpilation. That makes it a much more manageable thing to evaluate and shows the disruption to the dep tree is small. Thanks!

In this, if I've read correctly, the only new library outside postcss libraries is autoprefixer.

+++ b/core/package.json
@@ -21,6 +23,7 @@
   "devDependencies": {
+    "autoprefixer": "^9.6.1",
     "babel-core": "^6.26.0",
     "babel-plugin-add-header-comment": "^1.0.3",
     "babel-preset-env": "^1.4.0",
@@ -40,6 +43,10 @@

@@ -40,6 +43,10 @@
     "minimist": "^1.2.0",
     "mkdirp": "^0.5.1",
     "nightwatch": "^1.2.1",
+    "postcss": "^7.0.18",
+    "postcss-calc": "^7.0.1",
+    "postcss-custom-properties": "^9.0.2",
+    "postcss-header": "^1.0.0",
     "prettier": "^1.14.0",
     "stylelint": "^9.10.1",

So, let's do a dependenct evaluation for that one too. Thanks for your patience as i muddle through JS dep management things. :)

Oops, the needs followup tag was to separate documentation of core development toolchain vsl other dev res snd specific policies for them on https://www.drupal.org/core/dependencies#dev

lauriii’s picture

The reason for not listing autoprefixer in #23 is that we already have implicit dependency on autoprefixer through stylelint. I'm happy to provide an evaluation on that as well if we think that would be useful ✌️

finnsky’s picture

FileSize
179.7 KB

Nice initiative. +1

I found some places where we can to optimise things:
1) For example some scripts for js and css are equal. Like ChangeOrAdded.js
diff
Difference only in file ext. So we can merge common scripts.

2) We have few environments settings in package json now. It looks wierd imo. Also we have browserlist config line but `browserlist` not added in dependencies. Much better will add it in dependencies. use its power in environments configuration. and remove that settings from package.json.
https://github.com/browserslist/browserslist#configuring-for-different-e...

finnsky’s picture

Updated patch with described in #29 approach.

alexpott’s picture

Thanks for the suggestions @finnsky. I think the refactors make sense in someways but doing them should be a separate issue so we can discuss and BC concerns and other requirements. Also @lauriii points out that #30 tightly couples the CSS and JS build process. While CSS is brand new perhaps that will paint us into a corner we don't want to be in.

lauriii’s picture

#29.1 I kept the CSS build process separate from the JS on purpose. Having them defined as separate pipelines allows us to make changes to these two build tools individually. If we make both build tools to use the same abstracted tools, it forces someone making changes to CSS build to also have to worry about the JS build tools. These are also fairly simple files so the overhead from duplicating this code is not that high.

#29.2 I removed the duplicated browserslist configuration but I'm not sure we should be defining an explicit dependency on browserslist since we are not actually directly depending on it anywhere.

finnsky’s picture

RE: #32 #31 Actually build processes decoupled. Merged only file system processes. Both js and css build tools have own config and plugins list. And still independent from each other.

lauriii’s picture

@finnsky Do you think we could open a follow-up to discuss the abstractions part?

finnsky’s picture

Re: #34
Yes. Because build/watch are filesystem tasks. No matter what they build. Css/js/images/icons etc.
---
Also would be nice to create follow-up about usage of .babelrc and postcss.config.js to disquss abilities of modules and themes to use own plugins and configs with core build tools.
Thanks.

lauriii’s picture

lauriii’s picture

Opened follow-up #3084850: Consider abstracting some parts out of JavaScript / CSS build tools for the abstraction changes proposed in #29.

lauriii’s picture

Issue tags: -Needs followup
lauriii’s picture

Issue summary: View changes
Issue tags: -Needs issue summary update
lauriii’s picture

Issue summary: View changes
lauriii’s picture

Issue tags: -Needs change record
Wim Leers’s picture

  1. +++ b/core/package.json
    @@ -21,6 +23,7 @@
    +    "autoprefixer": "^9.6.1",
    
    @@ -68,21 +75,19 @@
    -              "targets": {
    -                "browsers": [
    -                  "ie >= 9",
    -                  "edge >= 13",
    -                  "firefox >= 5",
    -                  "opera >= 12",
    -                  "safari >= 5",
    -                  "chrome >= 56"
    -                ]
    -              }
    +              "modules": false
                 }
               ]
             ]
           }
         }
    -  }
    +  },
    +  "browserslist": [
    +    "ie >= 9",
    +    "edge >= 13",
    +    "firefox >= 5",
    +    "opera >= 12",
    +    "safari >= 5",
    +    "chrome >= 56"
    

    👍

    The list of targeted browsers remains the same, it is just being moved. Rather than it being passed specifically to babel only, we're making this the official browserslist that any JS package is being told to target.

    Why are we doing this? Because autoprefixer also uses this.

  2. +++ b/core/package.json
    @@ -40,6 +43,10 @@
    +    "postcss-header": "^1.0.0",
    

    👍
    This is the only remotely concerning JS dependency, because it has fewer installs/users. But … it's absolutely trivial, so the potential burden on us (the Drupal community) to maintain this ourselves is minimal and trivial.

    We want/need this for the "DO NOT EDIT THIS FILE" header at the top of the CSS files built/processed by PostCSS (*.pcss.css*.css). Just like we do for JS files (*.es6.js*.js).

  3. +++ b/core/scripts/css/changeOrAdded.js
    --- /dev/null
    +++ b/core/scripts/css/check.js
    

    🤔 These files are 100% identical to the identically named files in core/scripts/js/. Any way we can reuse rather than duplicate?

  4. +++ b/core/scripts/css/compile.js
    @@ -0,0 +1,34 @@
    +        preserve: false,
    ...
    +        cascade: false
    

    🤔 Let's document the reasons for these flags.

  5. +++ b/core/scripts/css/compile.js
    @@ -0,0 +1,34 @@
    +        header: `/*\n * DO NOT EDIT THIS FILE.\n * See the following change record for more information,\n * https://www.drupal.org/node/2815083\n * @preserve\n */\n`,
    

    👍 The reason for this is obvious.

  6. +++ b/core/scripts/css/postcss-build.js
    @@ -0,0 +1,49 @@
    + * yarn run build:css -- --file misc/drupal.pcss.css --file misc/drupal.init.pcss.css
    ...
    +const fileMatch = './**/*.pcss.css';
    

    🤔 This is the only change, can we make this show up as a copy + modify in the git-generated patch?

    🥳 Found it! Generate a patch using git diff -C -C (that is not a typo). This does not make the patch smaller, but does it make it significantly easier to review, because it makes it very obvious that this code is following an already established pattern.

  7. +++ b/core/yarn.lock
    @@ -1426,6 +1431,16 @@ cross-spawn@^6.0.5:
    +css-unit-converter@^1.1.1:
    

    👍 A lot of additions here, but yarn why explains all of them: they are dependencies of the postcss packages we're adding.

  8. 🤔 It's great that this adds yarn run watch:css, which is similar to yarn run watch:js. But what if I want both at the same time? I want a single command to watch and build all the things.
Wim Leers’s picture

Status: Needs review » Needs work

Marking Needs work for points 3, 4 and 8.

lauriii’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
23.11 KB
5 KB

Thank you for the review @Wim Leers 🙏

#42.3 We already have a follow-up for this: #37.
#42.4 👍
#42.8 👍

Wim Leers’s picture

  • #42.3: 👍 Added as related issue.
  • #42.4: 👍
  • #42.8: why only yarn run watch and not also yarn run build?
  • #44 also added postcss-import. Why are we adding this now? Oh you updated the issue summary! 👏
lauriii’s picture

Good idea! Added yarn build as well.

Wim Leers’s picture

#46: Excellent! Now I can just change that terminal tab from yarn run watch:js to yarn run watch and still have everything being built for me! 🥳

I took one last hard look at this issue. I found only one remaining question before I'd RTBC this:

  1. Though perhaps yarn run watch:all and yarn run build:all would be clearer? Are there conventions for this? I'm fine with this as-is, but if there is a front-end developer community convention around this, we should go with that.
lauriii’s picture

I've never seen yarn run watch:all myself. 🤔 I made a quick Google search with keywords site:github.com "yarn watch" and site:github.com "yarn watch:all" which confirm that there seems to be only a handful of libraries using yarn watch:all. In comparison, there's tens of pages that can be found with the search looking for yarn watch. I'm personally fine to go either way, but yarn watch felt like the natural way to provide a watch-all-the-things command.

Wim Leers’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Sounds good!

I expanded the issue summary to preemptively answer the committer question So now I need to run not only yarn run watch:js but also yarn run watch:css, doesn't this harm DX? 🤓

Wim Leers’s picture

Title: Proposal to use PostCSS for Claro in core » Use PostCSS in core, initially only for Claro
Category: Plan » Feature request
Issue summary: View changes

This is no longer just a plan, nor is it tightly coupled to Claro. The issue summary already reflects this. The issue title did not yet.

Wim Leers’s picture

Issue tags: +blocker
xjm’s picture

The CR should mention that, for the time being, this only applies to the new experimental Claro theme. Otherwise it sounds way more disruptive than it is.

We should also add a PR to the committer toolchain to check claro theme files (once they exist) in the pre-commit hook and verify the build output matches, as is done with JS.

xjm’s picture

Also, I need links to and documentation of Autoprefixer's security policies/contacts, release policies, even the repo, etc. Thanks!

xjm’s picture

Status: Reviewed & tested by the community » Needs work
lauriii’s picture

I updated the CR.

#53: Autoprefixer is part of the PostCSS toolchain. They follow the same policies as the main repository and they also use Tidelift for reporting security issues.

Still need to open PR for the committer tooling.

lauriii’s picture

If anyone else is interested in working on the committer tooling improvement, the repository for that is here: https://github.com/alexpott/d8githooks .

Wim Leers’s picture

Assigned: Unassigned » Wim Leers

#56: On it.

Wim Leers’s picture

Assigned: Wim Leers » Unassigned
Status: Needs work » Reviewed & tested by the community
Wim Leers’s picture

alexpott’s picture

@Wim Leers's merge on githooks revealed this patch has a tiny issue :)

alexpott’s picture

FileSize
457 bytes

ughh wrong interdiff.

lauriii’s picture

+++ b/core/scripts/css/postcss-watch.js
--- a/core/themes/seven/css/base/elements.css
+++ b/core/themes/seven/css/base/elements.css

+++ b/core/themes/seven/css/base/elements.css
@@ -1,3 +1,9 @@
+/*
+ * DO NOT EDIT THIS FILE.
+ * See the following change record for more information,
+ * https://www.drupal.org/node/2815083
+ * @preserve
+ */
 /**
  * Generic elements.
  */
@@ -33,7 +39,6 @@ summary,

@@ -33,7 +39,6 @@ summary,
 .simpletest-results-form summary {
   text-transform: none;
 }
-
 /**
  * Reusable heading classes are included to help modules change the styling of
  * headings on a page without affecting accessibility.

similarity index 100%
copy from core/themes/seven/css/base/elements.css

copy from core/themes/seven/css/base/elements.css
copy to core/themes/seven/css/base/elements.pcss.css

I was planning to remove these changes from the patch before this actually gets committed. This was here just for demonstration purposes. I guess the previous comments prove that it works as expected?

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Yep! I thought I'd just learned that we already had a .pcss.css file in HEAD :D lolz... okay yeah we should be removing that and the changes to seven's css. And also yay @Wim Leers's update to githooks look like they work fine.

lauriii’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
21.85 KB
523 bytes

Not sure why removing the .pcss.css file doesn't show up in the interdiff. 🤯Moving back to RTBC since my changes are insignificant.

Wim Leers’s picture

And also yay @Wim Leers's update to githooks look like they work fine.

🥳

alexpott’s picture

As far as I can see all the release manager feedback has been addressed and this is ready for commit. Crediting @xjm and @catch for release management comments. Will wait for them to confirm or remove the tag before committing.

catch’s picture

I think all of xjm's feedback has been addressed, so removing the needs release manager review tag.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 0bada78 and pushed to 8.8.x. Thanks!

  • alexpott committed 0bada78 on 8.8.x
    Issue #3060153 by lauriii, alexpott, finnsky, Wim Leers, xjm, catch: Use...
Wim Leers’s picture

Published the change record! 🥳

davidhernandez’s picture

Should this be marked fixed without a full documentation page in the contribution guide? This is the first of its kind.

saschaeggi’s picture

Once in core, can we also start using the nesting plugin for postCSS (SCSS-like syntax):
https://github.com/jonathantneal/postcss-nesting
That could also save a lot of time rewriting CSS.

catch’s picture

Status: Fixed » Needs work

Back to needs work until the contributor documentation is up.

droplet’s picture

.pcss.css isn't a well-compatible file extension for popular editors, eg. VSCode & PHPStorm. While this is still fresh, I think the community should make a good decision immediately to reduce code debt.

lauriii’s picture

#74: Code editors read .pcss.css as .css by default. We are using PostCSS for generating CSS that's supported by older browsers from modern CSS. This means that we don't really need any special IDE handling for these files in core. Since the tooling is internal and designed to be used only by core, contrib projects willing to use PostCSS for something else could continue to use any other file extension as they wish.

xjm’s picture

+++ b/core/package.json
@@ -68,21 +78,19 @@
+  "browserslist": [
+    "ie >= 9",
+    "edge >= 13",
+    "firefox >= 5",
+    "opera >= 12",
+    "safari >= 5",
+    "chrome >= 56"

Hmm, why doesn't this reflect our new browser policy? (as I reviewed it in another issue recently).

Also I heard someone say in backchannel that this patch uses an unsupported release if PostCSS; is that true? If so we need to discuss that and possibly revert, or get more info from the maintainers about the issue with the current version.

lauriii’s picture

Hmm, why doesn't this reflect our new browser policy? (as I reviewed it in another issue recently).

We have an issue to solve that #3084843: Update browserslist browsers. It's not in the scope of this issue.

Also I heard someone say in backchannel that this patch uses an unsupported release if PostCSS; is that true? If so we need to discuss that and possibly revert, or get more info from the maintainers about the issue with the current version.

Core is not using an unsupported version of PostCSS, or any other libraries related to this. Claro is using a previous major version of one of the libraries and there was a regression on the most recent major release. I've opened an issue in the package to solve that and we've come up with a workaround for the problem in Claro. Before this research happened, it was on the table to use an older version of that library in core as well, but that never happened because we don't want to depend on an unsupported version of a library, and we already found a better workaround for the problem.

droplet’s picture

#75

Am I right this only enables CSS4 CUSTOM VAR?

I've already seen suggestions coming up #72.

+++ b/core/package.json
--- /dev/null
+++ b/core/postcss.config.js

+++ b/core/postcss.config.js
@@ -0,0 +1,20 @@
+      importFrom: [
+        './themes/claro/css/src/base/variables.css'
+      ]

this config isn't really used? If not, `importFrom` should not specify for CORE

davidhernandez’s picture

Issue tags: +Needs documentation
lauriii’s picture

Issue tags: +Needs follow-up

Am I right this only enables CSS4 CUSTOM VAR?

We use it for autoprefixer and CSS custom properties, calc and import. None of this requires any special handling from IDEs.

this config isn't really used? If not, `importFrom` should not specify for CORE

😱That's a mistake. It's not actually needed. Let's open a follow-up to remove that.

zrpnr’s picture

I wasn't sure where to add this documentation, I found Javascript Developer tools for Drupal core which already had info about installing yarn and running the js build commands.

I added the new commands for css as well as some intro text to explain JS and CSS preprocessing. I copied some passages from the respective change records as well since they had good examples of how and why to run these commands.

It might be better to have a separate page, but on the other hand this is part of the same tooling and managed the same way as js.

Let's open a follow-up to remove that.

#80
Opened a followup to remove the claro path in postcss.config.js
#3086931: Remove unused postcss.config.js

lauriii’s picture

Status: Needs work » Fixed
Issue tags: -Needs documentation, -Needs follow-up

I moved the documentation under Drupal 8 from JavaScript coding standards. I also changed the title to frontend development tools which is more accurate. Marking this as fixed since #79 and #80 has been addressed.

xjm’s picture

Status: Fixed » Needs work

Ah, part of the needed documentation was for the dependency itself, on this page:
https://www.drupal.org/core/dependencies#dev

Thanks!

lauriii’s picture

Status: Needs work » Fixed

Added documentation to the core dependencies documentation! Moving back to fixed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

jhodgdon’s picture

I found small a bug in the way this was done and created a child issue; posting here even though the issue is closed, in case anyone is following this.
#3155159: Fix notice at top of generated CSS files from PCSS