webpack-contrib / stylus-loader

:art: A stylus loader for webpack.
MIT License
498 stars 99 forks source link

[feedback] future of stylus-loader #229

Closed michaelalhilly closed 4 years ago

michaelalhilly commented 4 years ago

Reporting this issue here at the request of @evilebottnawi.

We need the following stylus options, which are currently not available in this loader:

  1. Define
  2. DefineRaw

Define makes an object available to all compiled Stylus files. This is useful for sharing data such as color values across all files HTML templates (i.e. Pug), JS, Stylus, etc.

If DefineRaw is true then the object is converted to a hash, else it is passed as a list/expression.

These options are currently available with Stylus-Native-Loader: https://github.com/slightlyfaulty/stylus-native-loader

Additionally, if it is possible it may be better practice to expose all of the Stylus options. Please see the Stylus-Native-Loader readme for more info.

@slightlyfaulty Please add anything I may have left out.

Thanks!

slightlyfaulty commented 4 years ago

Looks good. Thanks for adding this @michaelalhilly.

I believe define is already available, it's just not documented. Raw defines are much more useful though and should definitely be added in.

Related: #170

(FYI the link to stylus-native-loader seems to be broken)

michaelalhilly commented 4 years ago

@slightlyfaulty Fixed the link. Thanks!

Yes, I'm using the rawDefine option. @acidjazz is passing an object to the rawDefine option.

Is that working as expected? Or is it being passed in a rawDefine false?

alexander-akait commented 4 years ago

Code is very ugly here, need full rewrite :disappointed:

slightlyfaulty commented 4 years ago

@acidjazz's implementation is to have two options, define and rawDefine, one for standard defines and the other for raw defines.

In stylus-native-loader, all defines are added to define, and defineRaw is used to toggle whether they are raw or not (defaulting to true).

Since raw defines only affect objects, and 99% of the time you want a Stylus hash instead of a list, having 2 define options seems a bit redundant.

slightlyfaulty commented 4 years ago

@evilebottnawi Well it was released in 2013. Those were different times 😄

michaelalhilly commented 4 years ago

@evilebottnawi I see support for this in index.js now.

https://github.com/webpack-contrib/stylus-loader/blob/4e68a5fa364d652b68a602fa242a02c8bdd4b659/src/index.js#L95

I'm using 3.0.2. Is this suppose to be working?

I've got the following options on the loader:

loader: '/var/www/node_modules/stylus-loader/index.js',
options: {
  sourceMap: false,
  preferPathResolver: 'webpack',
  defineRaw: true,
  define: {
    global: {
      colors: {
        primary: '#26a69a'
      }
    }
  }
}

Unfortunately this is still not working for me.

FYI, the reference to the loader looks that way because this is being generated by Vue CLI.

slightlyfaulty commented 4 years ago

@michaelalhilly I don't think the update has been released yet. 3.0.2 is still the old version from 2018.

michaelalhilly commented 4 years ago

@slightlyfaulty Yes you're right. I see the 3.0.2 versions also has the define property. I logged the rendered output right before the render is invoked on the template and I can see the [expression] assigned to globals. It's just not accessible in the template.

I wonder if Vue CLI is causing a problem but I'm too tired to check separately. I'll try again tomorrow.

FYI, I tried using stylus-native-loader with Vue CLI and it did not work . If you're interested I can provide error output.

slightlyfaulty commented 4 years ago

@michaelalhilly I have tests for Vue components in stylus-native-loader, but I don't think I ever tried with Vue CLI. If you can please post your error output in a new issue it would be much appreciated.

alexander-akait commented 4 years ago

@slightlyfaulty @michaelalhilly This week we are planning to release a new version, I would be glad if you tested and review it :star: In our long term - have one maintainable loader, I will tell you as soon as the work is finished finally

slightlyfaulty commented 4 years ago

@evilebottnawi Looking forward to trying it out 👍

michaelalhilly commented 4 years ago

@evilebottnawi Awesome and yes! I took a look at master and it looks so much better than than the older 3.0.2. Much cleaner and easier to read. Nice work!

@slightlyfaulty Will post the error output later today on your repo so you can manage it there.

Thanks so much guys!