Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
5 of 6 tasks
oxyc opened this issue Jun 23, 2017 · 12 comments
Closed
5 of 6 tasks

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

oxyc opened this issue Jun 23, 2017 · 12 comments

Comments

@oxyc
Copy link
Contributor

oxyc commented Jun 23, 2017

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
Copy link
Member

QWp6t commented Jun 23, 2017

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
Copy link
Contributor Author

oxyc commented Jun 23, 2017

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

@myleshyson
Copy link

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
Copy link
Contributor Author

oxyc commented Jul 10, 2017

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

@QWp6t
Copy link
Member

QWp6t commented Jul 10, 2017

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
Copy link

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
Copy link

mtx-z commented Aug 21, 2017

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

@myleshyson
Copy link

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
Copy link

@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.

@retlehs retlehs added the bug label Apr 25, 2018
@kimhf
Copy link
Contributor

kimhf commented Apr 28, 2018

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
Copy link

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

@Log1x
Copy link
Member

Log1x commented Jun 18, 2019

#2172

@Log1x Log1x closed this as completed Jun 18, 2019
@Log1x Log1x removed the bug label Jun 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants