Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • 02:32

    ai on ose

    Regenerate Yarn lock (compare)

  • 02:28

    ai on ose

    Remove sharec config (compare)

  • 01:39

    ai on ose

    Improve custom properties parse… Update postcss-parser-tests (compare)

  • 01:13

    ai on ose

    Use unreleased postcss-parser-t… (compare)

  • Mar 28 23:06
    ai milestoned #1343
  • Mar 28 23:06
    ai labeled #1343
  • Mar 28 23:06
    ai opened #1343
  • Mar 28 23:05
    ai labeled #797
  • Mar 28 23:05
    ai labeled #503
  • Mar 28 23:02
    ai closed #1120
  • Mar 28 23:02
    ai commented #1120
  • Mar 28 23:01
    ai labeled #1305
  • Mar 28 23:01
    ai unlabeled #1324
  • Mar 28 23:01
    ai unlabeled #1305
  • Mar 28 23:01
    ai unlabeled #1299
  • Mar 28 23:01
    ai unlabeled #797
  • Mar 28 23:01
    ai unlabeled #1120
  • Mar 28 23:01
    ai unlabeled #296
  • Mar 28 23:01
    ai labeled #1324
  • Mar 28 23:01
    ai labeled #1299
Jonathan Neal
@jonathantneal
@evilebottnawi, if you can see a better way to do this, please let me know. https://github.com/preactjs/preact-cli/pull/832/files
Evilebot Tnawi
@evilebottnawi
@jonathantneal hm, you can create postcss.something.config.js and use require/import, it is reduce lines in config or we can implement options like config: false what disable loading user defined configuration
Jonathan Neal
@jonathantneal
I’m not sure that gives me what I was looking for. I want to the configuration file plugins to win over postcss-loader. It’s almost like I want the loader option to be defaultPlugins, which would only be used if plugins could not be found in the configuration file.
Evilebot Tnawi
@evilebottnawi
@jonathantneal hm, some understand, you want merge plugins from postcss.config.js (if exists) with postcss-loader plugins options, right?
Jonathan Neal
@jonathantneal
Close. However, I do not want to merge both arrays of plugins.
It’s like this — configFilePlugins || someDefaultPlugins — use the configuration file plugins, but if they are not specified then use the default plugins.
@evilebottnawi, this allows the configuration file to override the plugins option in postcss-loader.
Evilebot Tnawi
@evilebottnawi
hm, it is works in other way? It should try to load config, it config not found use someDefaultPlugins
Jonathan Neal
@jonathantneal

@evilebottnawi, may it help to see the postcss-loader options? Maybe this will clarify: https://github.com/preactjs/preact-cli/blob/master/packages/cli/lib/lib/webpack/webpack-base-config.js#L206-L213

And here is the change I want to make: https://github.com/preactjs/preact-cli/blob/00bb61f865917db108ef90974bca291d5d5864aa/packages/cli/lib/lib/webpack/webpack-base-config.js#L207-L224

If plugins option for postcss-loader looks like this — { loader: 'postcss-loader', options: { plugins: [ autoprefixer({ overrideBrowserslist: browsers }) ] } } — then it will override the plugins option in postcss.config.js.
Jonathan Neal
@jonathantneal
This behavior ultimately comes from postcss-load-config, https://github.com/michael-ciniawsky/postcss-load-config/blob/master/src/options.js#L40-L42
Evilebot Tnawi
@evilebottnawi
yes, oh it is bug, we should use logic as you written above, @ai what do you think?
can you open issue in postcss-loader?
We should fix behavior and release major version with fix
Jonathan Neal
@jonathantneal
I’m not sure what I’m suggesting should be the default behavior. I just want the ability to override the loader plugins. It would be much easier if there were just another option to specify defaultPlugins.
Evilebot Tnawi
@evilebottnawi
why? you already have plugins option, logic should be:
  1. try to search and load user configuration
  2. if not found use plugins option
Jonathan Neal
@jonathantneal
Maybe. Hmm. I’d like to know what @ai thinks. :point_up:
Benedikt Gregor
@scsskid
Hi, I'm wondering about the features option, why do I have to (?) additionally set options likenesting-rules' : truewhen installing and require a plugin (like postcss-nestedor postcss-nesting
Jonathan Neal
@jonathantneal
Here is my PR to add support for postcss.config.js in Preact CLI, which will let us specify our own PostCSS plugins without overriding the core preact configuration. preactjs/preact-cli#832
Evilebot Tnawi
@evilebottnawi
@jonathantneal good workaround, but i think we should fix logic on postcss-loader side, can you open issue?
Jonathan Neal
@jonathantneal
I will open an issue. Thanks for reviewing the change. I hope the maintainers approve it!
Evilebot Tnawi
@evilebottnawi
i am maintainer of postcss-loader :smile:
Sam Smith
@smth

Hi. I've run into a problem a couple of times now. I'm hoping someone might be able to shed some light on it. This seems to only occur once the codebase gets large. I don't have a minimal example, but I'll describe what one might look like.

File A has multiple (postcss-advanced-variables) mixin definitions.
File B calls one of those mixins, and passes parameters.
File C, calls another.
File D (postcss-import) imports file A, file B, and file C, and gets outputted.

When the problem arises, changes to parameters in file C for example have no effect on the output from file D. It feels as though something is being cached somewhere, to the point that even deleting the mixin in file A has no effect on the output. At this point I would expect there to be an error, because I'm calling a mixin that doesn't exist, but all gulp processes appear to run fine, and I can continue to edit file B and see those changes reflected.

I don't understand what's happening here. Any ideas?

Sam Smith
@smth
I should add that I'm using postcss-custom-properties too.
Sam Smith
@smth
Stopping the Gulp process can result in the expected error. I would then expect at this point, if there was some caching responsible for the strange behaviour, it will now have been cleared. However, reinstating the mixin again results in an output with old values (values that no longer exist anywhere in the codebase).
Jonathan Neal
@jonathantneal
This is really random, @evilebottnawi, but I noticed a very reversion in style-loader that I’m not sure was intended.
Specifically, it was this work from last year: webpack-contrib/style-loader#219
That PR clearly shows that the work was approved and merged. However, the work was merged into a webpack-contrib:next branch on May 22, 2018.
Evilebot Tnawi
@evilebottnawi
oh, style-loader in my todo, but not near future
maybe next month
Jonathan Neal
@jonathantneal
Maybe I can help?
Jonathan Neal
@jonathantneal
Fix submitted, @evilebottnawi webpack-contrib/style-loader#382
With this fix, we can have source map support in Create React App again. :tada:
Jonathan Neal
@jonathantneal
@evilebottnawi what can I do to help here? I know there has been a refactor since the last version — do you need more tests? Is there a project board?
Evilebot Tnawi
@evilebottnawi
@jonathantneal thanks for helping, no desk, just need tests and refactor code on separate files, or we can merge this, drop old node and release next version, and do other things in 2 version :smile:
Jonathan Neal
@jonathantneal

tests and refactor code on separate files

@evilebottnawi, is this something I can help with? As far as I know, the tests are in separate files from the functional code already. Maybe I am misunderstanding you?

Evilebot Tnawi
@evilebottnawi
@jonathantneal oh, i think we need refactor https://github.com/webpack-contrib/style-loader/blob/master/src/addStyles.js to separate files like utils/insertStyleElement.js, utils/removeStyleElement.js, and test them better
Jonathan Neal
@jonathantneal
Right. That makes sense. With the fix in 382, would it be a good idea to release that, and then perform the refactor separately? It would seem better to me, only because then we are not introducing too many changes in a single release.
Evilebot Tnawi
@evilebottnawi
@jonathantneal 383 :smile: yep, but will be great do refactor before release next version, sometimes you do regression when refactor, will be great catch them after release, because some regressions require new major release
Jonathan Neal
@jonathantneal
Oh right, 383. :smile:
Jonathan Neal
@jonathantneal
One thing I’ve noticed is that I cannot npm link within style-loader and then npm link style-loader in a project and verify the changes. I am wondering if something about the dist is now different?
This is just a theory, but I wonder if you already did a refactor since the last release, and did not update the package.json files field, maybe?
Jonathan Neal
@jonathantneal
What is the ose branch used for, @ai?
Is that the next version? Will it be major? minor? patch?
Andrey Sitnik
@ai
@jonathantneal 7.0, next major release (drop Node.js 4, 6, use ES6 without Babel, add visitor API)
Jonathan Neal
@jonathantneal
Awesome! Will you publish an @next version of the beta or RC before the full release?
I would love to rework my plugins before it is fully released.
Andrey Sitnik
@ai
I found performance regression and we are writing docs about creating plugins
when these tasks will be finished we are ready for release
To be honest, you do not need @next to test a new major update. Since we do not use Babel anymore, you can use master from GitHub in dependencies and everything will work