ctf0 / laravel-mix-versionhash

Auto append hash to file instead of using virtual one
MIT License
61 stars 18 forks source link

mix-manifest.json incorrect file name key #14

Closed tpetry closed 5 years ago

tpetry commented 5 years ago

Keys within mix-manifest.json file for laravel-mix-versionhash@1.0.5 are missing some extension name characters. So somefile.css will be somefile.s for the mix-manifest.json key. It's doing this for every file:

{
  "/js/app.s": "/js/app.cad03a0e.js",
  "/js/iframeResizer-contentWindow.s": "/js/iframeResizer-contentWindow.969c49fe.js",
  "/css/app.s": "/css/app.75623058.css",
  "/css/tinymce-editor.s": "/css/tinymce-editor.75623058.css"
}
ctf0 commented 5 years ago

this is related to https://github.com/ctf0/laravel-mix-versionhash/commit/5b3707bf856ca26336f2cdd28710175789a030d8

which is for the new version of laravel-mix aka.v4, so which version r u using ?, also can u try to manually change hash to contenthash and reply back ?

bolechen commented 5 years ago

+1 same error break on my production :'(

webpack.mix.js is below:

let mix = require('laravel-mix');

require('laravel-mix-sass-resources-loader')
// require('laravel-mix-purgecss')
require('laravel-mix-versionhash')

// const BundleAnalyzerPlugin = require('webpack-bundle-analyzer').BundleAnalyzerPlugin

/*
 |--------------------------------------------------------------------------
 | 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 the application as well as bundling up all the JS files.
 |
 */

mix.js('resources/assets/js/app.js', 'public/js').extract(['vue'])
    .sass('resources/assets/sass/app.scss', 'public/css')
    .sass('resources/assets/sass/admin.scss', 'public/css')
    .disableNotifications();

//全局样式
mix.sassResources('resources/assets/sass/variables.scss');

if (mix.inProduction()) {
    // mix.purgeCss();
    mix.versionHash();
} else {
    mix.sourceMaps();
}

mix.webpackConfig({
    plugins: [
        // new BundleAnalyzerPlugin(),
    ],
    output: {
        publicPath: '/',
        chunkFilename: 'js/[name].[chunkhash].js',
    },
});

mix.options({
    processCssUrls: true,
    purifyCss: true,
    extractVueStyles: false,
    imgLoaderOptions: {
        enabled: false
    },
    postCss: [
        // require('postcss-pxtorem')({rootValue: 16, propList: ['*'] }),
    ],
    uglify: {
        uglifyOptions: {
            compress: {
                drop_console: true,
                drop_debugger: true
            }
        }
    }
});
tpetry commented 5 years ago

I am using laravel-mix@4.0.14 and laravel-mix-versionhash@1.0.5.

Changing hash to contenthash does not solve the problem, the following error will be thrown:

Error: Path variable [contenthash:8] not implemented in this context: /css/app.[contenthash:8].css
    at fn (/Users/tpetry/Documents/laravel/node_modules/webpack/lib/TemplatedPathPlugin.js:43:11)
    at fn (/Users/tpetry/Documents/laravel/node_modules/webpack/lib/TemplatedPathPlugin.js:31:16)
    at String.replace (<anonymous>)
    at replacePathVariables (/Users/tpetry/Documents/laravel/node_modules/webpack/lib/TemplatedPathPlugin.js:99:5)
    at SyncWaterfallHook.eval [as call] (eval at create (/Users/tpetry/Documents/laravel/node_modules/tapable/lib/HookCodeFactory.js:19:10), <anonymous>:7:16)
    at MainTemplate.getAssetPath (/Users/tpetry/Documents/laravel/node_modules/webpack/lib/MainTemplate.js:508:31)
    at Compilation.getPath (/Users/tpetry/Documents/laravel/node_modules/webpack/lib/Compilation.js:2478:28)
    at getPath (/Users/tpetry/Documents/laravel/node_modules/extract-text-webpack-plugin/dist/index.js:315:51)
    at ExtractTextPlugin.extractedChunks.forEach.extractedChunk (/Users/tpetry/Documents/laravel/node_modules/extract-text-webpack-plugin/dist/index.js:323:83)
    at Array.forEach (<anonymous>)
    at compilation.hooks.additionalAssets.tapAsync.assetCb (/Users/tpetry/Documents/laravel/node_modules/extract-text-webpack-plugin/dist/index.js:301:25)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/tpetry/Documents/laravel/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:12:1)
    at AsyncSeriesHook.lazyCompileHook (/Users/tpetry/Documents/laravel/node_modules/tapable/lib/Hook.js:154:20)
    at hooks.optimizeTree.callAsync.err (/Users/tpetry/Documents/laravel/node_modules/webpack/lib/Compilation.js:1307:32)
    at _err1 (eval at create (/Users/tpetry/Documents/laravel/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:23:1)
    at _async2.default.forEach.err (/Users/tpetry/Documents/laravel/node_modules/extract-text-webpack-plugin/dist/index.js:296:11)
    at /Users/tpetry/Documents/laravel/node_modules/async/dist/async.js:473:16
    at iteratorCallback (/Users/tpetry/Documents/laravel/node_modules/async/dist/async.js:1064:13)
    at /Users/tpetry/Documents/laravel/node_modules/async/dist/async.js:969:16
    at _async2.default.forEach.err (/Users/tpetry/Documents/laravel/node_modules/extract-text-webpack-plugin/dist/index.js:274:13)
    at /Users/tpetry/Documents/laravel/node_modules/async/dist/async.js:473:16
    at iteratorCallback (/Users/tpetry/Documents/laravel/node_modules/async/dist/async.js:1064:13)
    at /Users/tpetry/Documents/laravel/node_modules/async/dist/async.js:969:16
    at compilation.rebuildModule.err (/Users/tpetry/Documents/laravel/node_modules/extract-text-webpack-plugin/dist/index.js:261:26)
    at callback (/Users/tpetry/Documents/laravel/node_modules/webpack/lib/Compilation.js:1133:5)
    at processModuleDependencies.err (/Users/tpetry/Documents/laravel/node_modules/webpack/lib/Compilation.js:1156:5)
    at process.internalTickCallback (internal/process/next_tick.js:70:11)
tpetry commented 5 years ago

Like issue #11 the error is again within the logic for transforming the mix-manifest.json. Replacing the filename key is the culprit. It looks like the regular expression filename change is to error-prone, i think a more sophisticated logic is required

JSON before modification:

{
   "/js/app.cad03a0e.js":"/js/app.cad03a0e.js",
   "/js/iframeResizer-contentWindow.969c49fe.js":"/js/iframeResizer-contentWindow.969c49fe.js",
   "/css/app.75623058.css":"/css/app.75623058.css",
   "/css/tinymce-editor.75623058.css":"/css/tinymce-editor.75623058.css"
}

JSON after modification:

{
   "/js/app.s":"/js/app.cad03a0e.js",
   "/js/iframeResizer-contentWindow.s":"/js/iframeResizer-contentWindow.969c49fe.js",
   "/css/app.s":"/css/app.75623058.css",
   "/css/tinymce-editor.s":"/css/tinymce-editor.75623058.css"
}
ctf0 commented 5 years ago

@tpetry u r probably right, i would deeply appreciate it if you can help with fixing the regex issue which would fix multiple probs

tpetry commented 5 years ago

I debugged the code which is transforming the filenames and came to the conclusion that the regular expression is too unspecific. It's matching anything before the file extension which is the also the reason of #11.

The following regular expressions are solving this issue and #11 by being more specific:

const removeHashFromKeyRegex = new RegExp(delimiter + '\.([a-f0-9]+)\.(.+)$', 'g')
const removeHashFromKeyRegexWithMap = new RegExp(delimiter + '\.([a-f0-9]+)\.(.+)\.map$', 'g')

These regular expressions are searching for a hex character sequence (the structure of a hash) surrounded by dots in the filename.

ctf0 commented 5 years ago

@tpetry can u make a PR with the changes ?

tpetry commented 5 years ago

It‘s just replacing the old regexes with the new ones. Can make a pull request on thursday or friday.

ctf0 commented 5 years ago

i will do it then, are u sure it doesnt effect anything else ?

tpetry commented 5 years ago

Pretty sure. It‘s just a better regexp. But i currently see that you support configurable delimeters, will have to change the regex to support this.

ctf0 commented 5 years ago

i will wait for ur PR then