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

[Asset] PathPackage combined with JsonManifestVersionStrategy not sitable with laravel-mix manifest #36234

Closed
glensc opened this issue Mar 27, 2020 · 15 comments

Comments

@glensc
Copy link
Contributor

glensc commented Mar 27, 2020

Description

Laravel Mix provides a fluent API for defining Webpack build steps for your Laravel application using several common CSS and JavaScript pre-processors.

The mix manifest is created with a leading slash:

{
    "/dist/main.js": "/dist/main.js?id=ecfe06d840525bff34b2",
    "/dist/main.css": "/dist/main.css?id=5b13608ecbc9f614ba25"
}

This file is compatible with JsonManifestVersionStrategy,
however PathPackage to prepend path expects input to be without leading slash.

Example

// I expect to get as result: /example/dist/main.js?id=ecfe06d840525bff34b2

$manifestPath = 'mix-manifest.json';
$relativeUrl = '/example';
$versionStrategy = new JsonManifestVersionStrategy($manifestPath);
$package = new PathPackage($relativeUrl, $versionStrategy);

// returns /dist/main.js?id=ecfe06d840525bff34b2 because leading slash
var_dump($package->getUrl("/dist/main.js"));

// returns "/example/dist/main.js" because no match in manifest
var_dump($package->getUrl("dist/main.js"));
@glensc
Copy link
Contributor Author

glensc commented Mar 27, 2020

Altho I'd like to see a solution from Symfony/Asset side, I've updated on the Mix side to strip prefix from items.

Origin: glensc/eventum@6f38e33

const collect = require('collect.js');

/**
 * Update manifest to remove leading slash of key => value pairs
 * @author Elan Ruusamäe <glen@pld-linux.org>
 * @see https://github.com/symfony/symfony/issues/36234
 */
mix.extend('updateManifestPathsRelative', (config) => {
    config.plugins.push(new class {
        apply(compiler) {
            compiler.plugin('done', () => {
                const manifest = {};
                collect(Mix.manifest.get()).each((value, key) => {
                    key = this.normalizePath(key);
                    value = this.normalizePath(value);
                    manifest[key] = value;
                });
                Mix.manifest.manifest = manifest;
                Mix.manifest.refresh();
            });
        }

        /**
         * @param {string} filePath
         */
        normalizePath(filePath) {
            if (filePath.startsWith('/')) {
                filePath = filePath.substring(1);
            }

            return filePath;
        }
    })
});

mix.updateManifestPathsRelative();

@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@glensc
Copy link
Contributor Author

glensc commented Feb 20, 2021

image

@carsonbot carsonbot removed the Stalled label Feb 20, 2021
@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@glensc
Copy link
Contributor Author

glensc commented Aug 21, 2021

@carsonbot I don't see anyone from Symfony side triaged this.

@carsonbot carsonbot removed the Stalled label Aug 21, 2021
@mikiamomik

This comment has been minimized.

@GromNaN
Copy link
Member

GromNaN commented Jan 24, 2022

The described behavior was intentionally implemented (and fixed) by #22528. The base path is not prepend to asset paths that start with a /.

@glensc

This comment has been minimized.

@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@glensc
Copy link
Contributor Author

glensc commented Aug 29, 2022

I don't think any progress have been made here.

@carsonbot carsonbot removed the Stalled label Aug 29, 2022
@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@glensc
Copy link
Contributor Author

glensc commented Mar 5, 2023

No.

@carsonbot carsonbot removed the Stalled label Mar 5, 2023
@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@carsonbot
Copy link

Could I get a reply or should I close this?

@carsonbot
Copy link

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants