Set MIME type based on requested asset

[renaatdemuynck]

trafficstars

This fixes issue #200. MIME types are now determined based on the requested asset. Also fixed some tests that were failing on my Windows machine due to the use of slashes instead of DIRECTORY_SEPARATORs.

[renaatdemuynck]

Can this pull request be accepted?

[RWOverdijk]

I had a comment here, but I never clicked on comment... It must have been a slow day.

I don't know if I can, it looks very much like it will break for existing implementations (as you had to alter the tests).

[wshafer]

@renaatdemuynck - If I'm reading this right this changes the responsibility of figuring out the Mime type from the actual file, to the file being requested?

So if I map test.css to real file test.js (not sure why I'd do that but I could) the mime type coming back would be text/css instead of the actual application/javascript for the file?

Personally I'm never a fan of trusting the client but is there some benefit here to letting the client tell you what the server should be serving as the mimetype, verses the actual files themselves?

[wshafer]

Just saw the open issue here: https://github.com/RWOverdijk/AssetManager/issues/200 which answers my question

[RWOverdijk]

Merging this would be a bump to 2.0.0. I'm wondering if maybe this behavior should be made configurable instead. Thoughts?

[wshafer]

@RWOverdijk - I was going go ahead and put this into the v2 branch.

[RWOverdijk]

@wshafer I'd prefer that, too.

[renaatdemuynck]

Sorry, I missed the whole conversation. I just got back from vacation. Thanks for merging this into v2. @RWOverdijk: About the ability to configure, I believe the standard behavior should be the new one. You could make an option to revert to the old behavior for BC reasons, but I don't see a real need for it.

[wshafer]

This is going to be a major version bump. So I'll just make it the default behavior. It makes more sense because there's another change coming to mimetypes and this fits perfectly with that change as well.

Thanks for the help

[thestanislav]

This breaks my code, we can not mix css, less, sass files now :( but we should do it.

// some_config.php
[
    'resolver_configs' => [
        'collections' => [
            'assets/css/base.css' => [
                'assets/less.css',
                'assets/sass.css',
                'assets/other_file.css'
            ],
            'assets/less.css' => [
                'path/less.less',
            ],
            'assets/sass.css' => [
                'path/sass.less',
            ],

    ],
    'filters' => [
        'assets/less.css' => [
            [
                'service' => 'SomeLessCompiler',
            ]
        ],
        'assets/sass.css' => [
            [
                'service' => 'SomeSassCompiler',
            ]
        ]
    ],
]

This code will fail with error "... doesn't have the expected mime-type ...", but logically everything should be fine

[
    'resolver_configs' => [
        'collections' => [
            'assets/base.css' => [
                'assets/less.less',
            ],

    ],
    'filters' => [
        'assets/base.css' => [
            [
                'service' => 'SomeLessCompiler',
            ]
        ],
    ],
]

assets/base.css will output with mime type text/less, but actual type is text/css after compilation

[wshafer]

Thanks for the report.

I believe I addressed this issue with a different merge for v2, but I’ll test this on my end and make sure this use case is working before I tag it.

[wshafer]

@thestanislav - I have finally tagged v 2.0.0. I have also confirmed that the following config will work just fine.

<?php

return [
    'asset_manager' => [
        'resolver_configs' => [
            'collections' => [
                'assets/css/base.css' => [
                    'assets/less.css',
                    'assets/scss.css',
                ],
            ],
            'private_collections' => [
                'assets/less.css' => [
                    __DIR__.'/../../assets/less.less',
                ],
                'assets/scss.css' => [
                    __DIR__.'/../../assets/scss.scss',
                ],
            ],
        ],

        'filters' => [
            'assets/less.css' => [
                [
                    'filter' => 'Lessphp'
                ]
            ],

            'assets/scss.css' => [
                [
                    'filter' => 'Scssphp'
                ]
            ],
        ]
    ]
];

To update to the beta version see the upgrade guide here

Let me know if you encounter any other issues.