roots / sage

WordPress starter theme with Laravel Blade components and templates, Tailwind CSS, and a modern development workflow
https://roots.io/sage/
MIT License
12.71k stars 3.06k forks source link

Watch task does not support entry points with only CSS #1911

Closed oxyc closed 5 years ago

oxyc commented 7 years ago

Submit a feature request or bug report


What is the current behavior?

Adding an entry point in resources/assets/config.json with only a SCSS file does not inject the CSS using HMR.

What is the expected or desired behavior?

Adding an entry point with only a stylesheet should inject the CSS with HMR.


Bug report

When I began writing this issue I thought this related to secondary entry points in general but turns out that this only happens when it's an entry point without a script file. I guess this might be a webpack limitation? As this still works as expected when building the assets using yarn run build, but not when using yarn run start, I'll stick to filing this as a bug report.

Edit: Read https://medium.com/webpack/the-new-css-workflow-step-1-79583bd107d7 on the future of stylesheets as first-class modules in webpack

Please provide steps to reproduce, including full log output:

  1. Create a resources/assets/styles/secondary.scss file

    echo "body { background-color: #ccc; }" >| resources/assets/styles/secondary.scss

  2. Add a second entry point in resources/assets/config.json

    { "entry": { "main": [ "./scripts/main.js", "./styles/main.scss" ], "secondary": [ "./styles/secondary.scss" ], "customizer": [ "./scripts/customizer.js" ] }, .... ] }

  3. Enqueue the stylesheet in `app/setup.php

    add_action('wp_enqueue_scripts', function () { wp_enqueue_style('sage/main.css', asset_path('styles/main.css'), false, null); wp_enqueue_style('sage/secondary.css', asset_path('styles/secondary.css'), false, null); wp_enqueue_script('sage/main.js', asset_path('scripts/main.js'), ['jquery'], null, true); }, 100);

  4. Launch the watch process

    yarn run start

  5. Note that the background color is white rather than gray.

Notes:

Please describe your local environment:

WordPress version: 4.8

OS: macOS Sierra

NPM/Node version: 5.0.3/8.1.0

Where did the bug happen? Development or remote servers?

Development

Is there a related Discourse thread or were any utilized (please link them)?

https://discourse.roots.io/t/managing-scss-themes-with-webpack/9293

QWp6t commented 7 years ago

Thanks for taking the time to provide such a detailed issue report. This will definitely be helpful. I'll try to address this issue this weekend.

Does it work if you add an empty scripts/secondary.js file?

touch resources/assets/scripts/secondary.js

and:

  "entry": {
    "main": [
      "./scripts/main.js",
      "./styles/main.scss"
    ],
    "secondary": [
      "./scripts/secondary.js",
      "./styles/secondary.scss"
    ],
    "customizer": [
      "./scripts/customizer.js"
    ]
  }

If that works, then the quick fix is as simple as iterating over all entry points and appending a js file if there isn't one already.

oxyc commented 7 years ago

Unfortunately no. In addition to what you mentioned, I also have to enqueue the script for the stylesheet to get loaded.

myleshyson commented 7 years ago

Yea I'm having the same issue. I'm trying to have multiple css files to optimize the front end. The build command works fine but the watch command does not save multiple css files into the dist/ folder. Is there a fix coming soon for this? I'm having to run yarn build every time I update my stylesheets.

oxyc commented 7 years ago

@myleshyson afaik there's no fix. It's part of the roadmap for webpack but probably wont exist for a while:

QWp6t commented 7 years ago

It's still something I might be able to address via BrowserSync. BS writes its own script snippet that gets loaded onto every page, and I know that snippet can be customized. I think I could use that feature of BS to load the script that serves the styles; this would actually enable me to serve everything in memory, which would hopefully improve performance.

For now the workaround would be to enqueue the script file that gets generated during development.

Something like this should work:

if (WP_ENV === 'development')
    wp_enqueue_script('sage/secondary.js', asset_path('scripts/secondary.js'), [], null, true);
}

(To be clear, yes you want to load a SCRIPT. During normal yarn build, that script is empty, but during watch, that script is what loads your styles.)

myleshyson commented 7 years ago

I ended up just replacing the build system with Laravel Mix and it worked exactly as expected with browsersync. I have two files (main.css and critical.css) and they both compile with BS. I love Sage, and definitely not trying to say Sage should use Laravel Mix, but I guess I'm wondering whats different about the way the sage build system works vs Laravel Mix?

mtx-z commented 7 years ago

Do you have any snippet to provide about Mix "-isation" of the build ? Thx !

myleshyson commented 7 years ago

Hey @mtx-z sure thing.

I deleted the resources/assets/build/ directory and completely replaced package.json with the default Laravel package.json. I replaced some packages then installed via yarn.

Then I copied the webpack.mix.js and webpack.config.js to the root directory

cp node_modules/laravel-mix/setup/** ./

Then in config/assets.php I changed the manifest directory to mix-manifest.json instead of assets.json.

And that's it! Here is my webpack.mix.js file

let mix = require('laravel-mix');
let ImageminPlugin = require('imagemin-webpack-plugin').default;
let CopyWebpackPlugin = require('copy-webpack-plugin');
let imageminMozjpeg = require('imagemin-mozjpeg');

/*
 |--------------------------------------------------------------------------
 | Mix Asset Management
 |--------------------------------------------------------------------------
 |
 | Mix provides a clean, fluent API for defining some Webpack build steps
 | for your Laravel application. By default, we are compiling the Sass
 | file for your application, as well as bundling up your JS files.
 |
 */

mix.js('resources/assets/scripts/main.js', 'scripts/')
   .js('resources/assets/scripts/customizer.js', 'scripts/')
   .sass('resources/assets/styles/main.scss', 'styles/')
   .sass('resources/assets/styles/critical.scss', 'styles/')
   .copyDirectory('resources/assets/images', './dist/images')
   .setPublicPath('dist')
   .options({
     processCssUrls: false
   })
   .browserSync({
    proxy: 'keith.dev',
    files: [
      'dist/**/*',
      'resources/**/*'
    ]
   });

if (mix.inProduction()) {
    mix.version();
    mix.webpackConfig({
    plugins: [
      // Copy the images folder and optimize all the images
      new CopyWebpackPlugin([{
        from: './resources/assets/images',
        to: './images'
      }]),
      new ImageminPlugin({
        test: /\.(jpe?g|png|gif|svg)$/i,
        optipng: { optimizationLevel: 7 },
        gifsicle: { optimizationLevel: 3 },
        pngquant: { quality: '65-90', speed: 4 },
        svgo: { removeUnknownsAndDefaults: false, cleanupIDs: false },
        plugins: [imageminMozjpeg({ quality: 60 })],
     })
    ]
  })
}

Works like a charm.

webstractions commented 6 years ago

@myleshyson Kudos dude! I am using Laravel Mix on a non-Sage theme and you solved a few issues that I was trying to wrap my brain around.

kimhf commented 6 years ago

I solved this with adding in config.js (Before public-path.js is added.) Look for entery points with only styles, then add a script (fix-style-entrypoints.js) to all other entery points that have scripts.

if (argv.watch) {
  const styleEntrypoints = []

  const isScriptExt = (ext) => {
    return ['.js', '.jsx', '.ts', '.tsx'].includes(ext)
  }

  const isStyleExt = (ext) => {
    return ['.css', '.scss', '.sass', '.less'].includes(ext)
  }

  Object.keys(module.exports.entry).forEach(id => {
    const extnames = module.exports.entry[id].map(file => {
      return path.extname(file)
    })

    if (!extnames.some(isScriptExt) && extnames.some(isStyleExt)) {
      // This is a only styles entry point.
      styleEntrypoints.push(id)
    }
  })

  Object.keys(module.exports.entry).forEach(id =>
    module.exports.entry[id].unshift(path.join(__dirname, 'helpers/fix-style-entrypoints.js?settings=' + encodeURIComponent(JSON.stringify({ styleEntrypoints })))))
}

Then added resources/assets/build/helpers/fix-style-entrypoints.js where the scripts needed for the css entry points to work are added to the document.

/* eslint-env browser */
/* globals __resourceQuery, FIXING_STYLE_ENTERY_POINT */

if (typeof FIXING_STYLE_ENTERY_POINT === 'undefined' || !FIXING_STYLE_ENTERY_POINT) {
  FIXING_STYLE_ENTERY_POINT = true // eslint-disable-line no-global-assign
  if (__resourceQuery) {
    var querystring = require('querystring')
    var parsedQuerystring = querystring.parse(__resourceQuery.slice(1))
    var settings = JSON.parse(parsedQuerystring.settings)

    if (settings && settings.styleEntrypoints && settings.styleEntrypoints.length && document.styleSheets && document.styleSheets.length) {
      const styleSheets = document.styleSheets
      const styleEntrypoints = settings.styleEntrypoints

      for (var i = 0; i < styleSheets.length; i++) {
        const styleSheet = styleSheets[i]
        styleEntrypoints.forEach(id => {
          if (styleSheet.href && styleSheet.href.match(`styles/${id}.css`)) {
            const script = document.createElement('script')
            script.type = 'text/javascript'
            script.src = styleSheet.href.replace('styles', 'scripts').replace('.css', '.js')
            document.getElementsByTagName('head')[0].appendChild(script)

            console.log(`Injected script ${script.src}`)
          }
        })
      }
    }
  }
}

Only works as long as there is at least one script enqueued.

petersontubini commented 5 years ago

+1 ! I agree to replace it to laravel mix, a lot better.

Log1x commented 5 years ago

2172