rykener / django-manifest-loader

Simplifies webpack configuration with Django
https://django-manifest-loader.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
104 stars 12 forks source link

Path of dynamic chunks is relative #42

Open bluesurfer opened 3 years ago

bluesurfer commented 3 years ago

The chunks resulting from a dynamic import are incorrectly loaded into the page. Their path is relative to the page thus causing 404 errors. To solve this I try to set the publicPath in Webpack config as suggested in https://github.com/webpack/webpack/issues/7968

    output: {
        path: path.resolve(__dirname, 'dist'),
        publicPath: "/",
        filename: "[name].[contenthash].js",
    },

Unfortunately that raises a Django SuspiciousFileOperation:

The joined path (/main.9ce82b7ec94c17ac77f9.css) is located outside of the base path component
rykener commented 3 years ago

@bluesurfer thanks for this report. Do you have an example django project you can link me to where I can see this problem occurring? I'm not fully understanding the issue and will need to be able to replicate it to fix it. Cheers.

bluesurfer commented 3 years ago

@shonin sorry but I am not sure that this issue is related with django-manifest-loader. Webpack is still a little magical to me so I am not sure who to blame :)

Anyway I will write down my progress... maybe it will help someone

Consider that I am shifting my project from django-webpack-loader and I have something similar to this . Everything seem to work except when using dynamic imports i.e.:

import('./modules/foo').then(...)

I followed the basic_usage example - Webpack will create a chunk and will correctly place its path in the manifest. Unfortunately, when requested, its path is relative to the current page. For instance, if the manifest path for such chunk is dist/foo.js and I am visiting example.com/whatever/ then a request to example.com/whatever/dist/foo.js is made instead of example.com/dist/foo.js causing a 404 error.

I think I solve this by using a node server like in the django-webpack-loader example. The only "hack" I have to made is to add a hook for webpack-manifest-plugin in order to have the manifest.json file stored in the output_dir and not served by the node server.

class WriteManifestToFile {
    apply(compiler) {
        const {afterEmit} = getCompilerHooks(compiler);
        afterEmit.tap('WriteManifestToFile', (manifest) => {
            fs.writeFileSync('manifest.json', JSON.stringify(manifest));
        })
    }
}

Reference https://github.com/shellscape/webpack-manifest-plugin#compiler-hooks

knokit commented 3 years ago

@bluesurfer, perhaps changing the publicPath to point to the directory from where Django is serving the static files will solve the problem (if you're serving the static files through Django)

kierandarcy commented 3 years ago

I had the same problem which I was able to solve by updating webpack.config.js.publicPath and by providing a custom loader in my Django settings.

I solved the webpack issue by changing the publicPath to be the Django settings.STATIC_URL.

webpack.config.js

output: {
    /* This is the Django settings.STATIC_URL */
    publicPath: '/static/',
    filename: '[name].[contenthash].js',
    path: path.resolve(__dirname, 'dist'),
},

I then had the issue where the STATIC_URL was being written twice by the {% manifest %} template tags. I fix that by using this custom loader in my settings.py

settings.py

# django-manifest-loader
# Add this *after* you define STATIC_URL
import fnmatch
from manifest_loader.loaders import LoaderABC

class ManifestLoader(LoaderABC):
    static_url = STATIC_URL

    @classmethod
    def _get_value(cls, value):
        if value.startswith(cls.static_url):
            return value[len(cls.static_url) - 1 :]
        return value

    @classmethod
    def get_single_match(cls, manifest, key):
        return cls._get_value(manifest.get(key, key))

    @classmethod
    def get_multi_match(cls, manifest, pattern):
        matched_files = [
            file for file in manifest.keys() if fnmatch.fnmatch(file, pattern)
        ]
        return [cls._get_value(manifest.get(file)) for file in matched_files]

MANIFEST_LOADER = {
    "loader": ManifestLoader,
}

I can see this being a reasonably common issue. In my case, I came across the issue when font files (bundled with some SCSS) were added by webpack with their relative URL - adding STATIC_URL to publicPath fixes this problem. The manifest_loader loader then has to know to remove the STATIC_URL from the beginning of the paths in manifest.json.

knokit commented 3 years ago

No need to write a custom loader for this. If you're using WebpackManifestPlugin, you just need to configure the publicPath.. in your case it's probably something like this:

webpack.config.js

output: {
    /* This is the Django settings.STATIC_URL */
    publicPath: '/static/',
    filename: '[name].[contenthash].js',
    path: path.resolve(__dirname, 'dist'),
},

plugins: [
    new WebpackManifestPlugin({
        publicPath: 'dist/'
    })
],
kierandarcy commented 3 years ago

Thank you @knokit

I hoped something like that would work. I tried that, but I still need a custom loader to fix the problem.

If I have publicPath set (using either option above) my manifest.json looks something like this...

{
  "main.js": "/static/main.123hash123.js",
  "main.woff": "/static/hash123hash.woff",
  "my_project/frontend/src/fonts/my-font.woff": "/static/hash123hash.woff"
}

With the DefaultLoaderthat means that I get this...

{% manifest_match '*.js' '<script src="{match}"></script>' %}

...outputs...

<script src="/static/static/main.123hash123.js"></script>

My custom loader removes the leading /static/ (if it exists), so now I get...

{% manifest_match '*.js' '<script src="{match}"></script>' %}

...outputs...

<script src="/static/main.123hash123.js"></script>
bluesurfer commented 3 years ago

My previous comment was too overkill :)

In my case, I finally solved this issue with the following settings:

Webpack configuration

    ....
    output: {
          path: path.resolve(__dirname, "dist"),
          publicPath: "/static/",
          filename: "[name].[contenthash].js",
      },
    ....
    plugins: [
        new WebpackManifestPlugin({
            publicPath: ""
        }),
    ],

Django settings

STATIC_URL = "/static/"
STATICFILES_DIRS = [str(ROOT_DIR / "dist")]

So to me this issue is closed.. I'll let the maintainer @shonin to consider closing this.

kierandarcy commented 3 years ago

Thank you for the update @bluesurfer your solution fixed my issue too. Also, thank you @knokit - I made a small update and I now no longer need my custom loader.

I just needed to update my WebpackManifestPlugin like this

new WebpackManifestPlugin({ publicPath: "" })