Skip to content
This repository has been archived by the owner on Jul 30, 2022. It is now read-only.

Why custom scripts and not webpack for building styles? #12

Closed
chanar opened this issue Apr 1, 2019 · 15 comments
Closed

Why custom scripts and not webpack for building styles? #12

chanar opened this issue Apr 1, 2019 · 15 comments
Labels
help wanted Extra attention is needed

Comments

@chanar
Copy link
Contributor

chanar commented Apr 1, 2019

Currently the npm run watch takes at least 1 second with minimal setup and only on package. So my questions is why custom scripts runner?

yarn run v1.12.3
$ ./scripts/build-styling.sh
Processing packages/core/scss/switch-base.scss
Processing packages/core/scss/button-base.scss
Processing packages/core/scss/range-base.scss
Processing packages/core/scss/progress-base.scss
Processing packages/core/scss/checkbox-base.scss
Done in 1.18s.

watcher build complete!
@web-padawan
Copy link
Owner

That's a good point. I agree the existing setup should be improved, while keeping the overall idea of compiling CSS. One idea which came to my mind is using the following tools:

I would be glad to accept a PR in case someone is willing to help with that setup.

@web-padawan web-padawan added the help wanted Extra attention is needed label Apr 1, 2019
@chanar
Copy link
Contributor Author

chanar commented Apr 1, 2019

We have a project that we have been building for some time now. I used your other package https://github.com/web-padawan/lit-components and have customized this for our current needs.

The project now uses

scss with import styles from './component-name.scss'; in each of our LitElement components.
scss mixins imported with webpack using sass-resources-loader
Storybook playing nice (with our custom rules set copied to .storybook/webpack.config.js
Custom vaadin styles with adding compiled scss to head.

I think the current process is similar what lit-css-loader allows to do but we are importing scss directly, the webpack converts it to css on the fly and the raw-loader allows importing css to lit component.

Complete webpack rules

rules: [
    {
      test: /\.js$/,
      loader: 'babel-loader',
      
      exclude: /node_modules\/(?!(@webcomponents\/shadycss|lit-element|lit-html|@polymer|@vaadin|@lit)\/).*/,
      options: {
        cacheDirectory: true
      }
    },
    {
      test: /\.scss$/,
      use: [
        {
          loader: 'raw-loader'
        },
        {
          loader: 'sass-loader'
        },
        {
          loader: 'sass-resources-loader',
          options: {
            resources: ['./src/styles/mixins.scss', './src/styles/inject.scss']
          }
        }
      ]
    },
    {
      test: /\.css$/,
      use: [
        {
          loader: 'css-loader'
        }
      ]
    }
  ]
module.exports = {
  env: {
    development: {
      presets: [
        [
          '@babel/preset-env',
          {
            exclude: ['transform-classes'],
            modules: false,
            targets: {
              ie: 11
            }
          }
        ]
      ],
      plugins: [
        [
          '@babel/plugin-transform-runtime',
          {
            corejs: false,
            helpers: false,
            regenerator: true,
            useESModules: false
          }
        ]
      ]
    },
    regressions: {
      presets: [
        [
          '@babel/preset-env',
          {
            modules: false,
            targets: {
              chrome: 69
            }
          }
        ]
      ]
    }
  }
};

I can talk about my setup more in detail if interested/needed. I also created a sample setup just after giving some edits to your lit-components package. https://github.com/chanar/lit-scss-vaadin

@web-padawan
Copy link
Owner

Thanks for sharing, I will evaluate the options.

I would still prefer not to import .scss files directly in the codebase and keep it "pure" ES modules, in order to avoid using webpack as the only tool, but instead continue with current approach: webpack for storybook, pack (effectively, rollup under the hood) for publishing to npm.

@chanar
Copy link
Contributor Author

chanar commented Apr 1, 2019

lit-css-loader for Storybook and visual tests.. What is your idea for running styles in development without the need to generate build/dist?

@chanar
Copy link
Contributor Author

chanar commented Apr 1, 2019

Thanks for sharing, I will evaluate the options.

I would still prefer not to import .scss files directly in the codebase and keep it "pure" ES modules, in order to avoid using webpack as the only tool, but instead continue with current approach: webpack for storybook, pack (effectively, rollup under the hood) for publishing to npm.

By this did you mean that when building for production, you would generate .css and have components importing .css instead of .scss. Which would mean using scss compilation with whatever tool (although needs to be certainly rollup, webpack etc) in development mode and only when building for production, -style.js files would be generated similar to your scripts folder functionality.

if (env === 'dev') {
import styles from './styles/component.scss';
} else {
import styles from './styles/component.js'; // generated when production build called
}

@web-padawan
Copy link
Owner

By this did you mean that when building for production, you would generate .css and have components importing .css instead of .scss.

The generated CSS is always wrapped into /styles/component.js and I would like to keep the .scss an implementation detail, and avoid any environment checks in the source code while possible (as I'm not sure whether Rollup can recognise and remove them or not).

@lkraav
Copy link
Contributor

lkraav commented Apr 9, 2019

Based on #15 we learned a few things

  • node-sass is currently not an option (rgb() function fails)
  • dart-sass should be a working alternative, but we just recently discovered this, and will attempt integration tomorrow
  • scripts/build-styling.sh is massively unoptimal as-is, since builds all components on every CSS change - should it perhaps become incrementally smarter towards a smaller rebuild surface (only rebuild in the same directory as changes made), or is pursuing this a waste of effort anyway? Maybe more advanced toolchains, like discussed above, should be attempted right away.

I understand one of Aybolit goals is "tool-agnostic", which makes sense, but I'm not very confident in estimating what's the next logical step is here.

Can scripts/build-styling.sh become smarter, or will it soon run into next problems, such as "can it handle modifying a shared dependency + rebuild multiple affected packages", or is each package always living its own isolated life?

@web-padawan your thoughts, please?

@web-padawan
Copy link
Owner

is each package always living its own isolated life?

Yes, that's the current state of things. In the Aybolit setup, each package is a separate library. I agree this might not be optimal for more common case where each package is a component.

scripts/build-styling.sh is massively unoptimal as-is, since builds all components on every CSS change

I do understand that, and the current setup was meant to deliver an MVP. As I mentioned above, I will evaluate the options and try using dart-sass, as Ruby Sass reached its end of life.

Unfortunately, I haven't had any time to work on this project for more than 2 weeks due to personal reasons. Hope to get back on track this week and continue on this subject, as well as #11

@lkraav
Copy link
Contributor

lkraav commented Apr 10, 2019

lit-css-loader for Storybook and visual tests
rollup-plugin-lit-css for preparing files to run Pack when building a "dist" output

Is HMR possible with anything other than Webpack? It'd be useful to be able to hot-reload at least style changes in the browser, without needing full page reload.

https://medium.com/webpack/webpack-and-rollup-the-same-but-different-a41ad427058c and rollup/rollup#50 seem to point to "no".

@web-padawan
Copy link
Owner

Is HMR possible with anything other than Webpack?

AFAIK, using HMR is unlikely to work properly with web components because of duplicate Custom Element definitions. So I usually have to restart Storybook after doing a bunch of changes.

@lkraav
Copy link
Contributor

lkraav commented Apr 10, 2019

@chanar @web-padawan perhaps #18 also resolves this issue now.

As HMR is unlikely to work w/ custom elements, I see no specific reason to aim for further root level Webpack integration. Do you?

@chanar
Copy link
Contributor Author

chanar commented Apr 10, 2019

We still need to turn the whole pack into x.bundle.js with minifications and so on. Webpack has perfect toolset for this. Also, what browsers we need to consider? So probably @web-padawan has had in mind to configure build script. But it usually is a thing to do depending on individual build needs.

@lkraav
Copy link
Contributor

lkraav commented Apr 10, 2019

I think our bundling should happen at theme (already Webpacking via Laravel Mix) and app level (whatever your conversionxl/cxl-app repo does, or wants to do), but bundling is probably not needed in conversionxl/aybolit components monorepo.

OTOH it may make sense to have known bundles, available directly from components monorepo. I wonder what @web-padawan thinks for this scenario. Maybe we talk this through in https://gitter.im/aybolitjs/community

@web-padawan
Copy link
Owner

Regarding building minified output, there is a pika-plugin-minify which I have tried, but so far it looks like that plugin only covers index.js and not the separate files under dist-src.

I'm not inclined to use webpack for preparing dist of the Aybolit at the moment. Pack which I'm using is based on Rollup under the hood, which is a great tool to output ES modules.

Once webpack 5 with first-class ES modules support (as a compile target) is out, we can probably reconsider using it but right now it doesn't seem a proper choice.

Of course in your projects you can use webpack, Parcel or anything (and Rollup, too).

@chanar
Copy link
Contributor Author

chanar commented May 17, 2019

Closing issue as building styles now works properly

@chanar chanar closed this as completed May 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants