just-jeb / angular-builders

Angular build facade extensions (Jest and custom webpack configuration)
MIT License
1.14k stars 198 forks source link

Modify rule regex #257

Closed mistic100 closed 5 years ago

mistic100 commented 5 years ago

I need to change the loader used for SVG files.

Currently Angular CLI declares a rule on \.(eot|svg|cur|jpg|png|webp|gif|otf|ttf|woff|woff2|ani)$ and uses the file-loader.

Is there a solution to alter the Regex of a an existing rule with angular-builders ?

Previously I used ngx-build-plus and was able to alter the merge configuration with the "plugin" feature. The plugin look likes :

exports.default = {
    config: (cfg) => {
        const rules = cfg.module.rules;

        // Remove SVG from file-loader
        const fileLoader = rules.find(rule => {
            return rule.loader === 'file-loader';
        });

        if (!fileLoader) {
            throw 'Cannot find assets rule';
        }

        fileLoader.test = new RegExp(fileLoader.test.source.replace('|svg', ''), fileLoader.test.flags);

        // Add SVG to raw-loader
        rules.push({
            test  : /\.svg$/,
            loader: 'raw-loader',
        });

        return cfg;
    }
};

If this is not currently possible with angular-builders would you accept a Pull request to add such feature ?

just-jeb commented 5 years ago

Thank you for taking time and opening this issue!

I'm pretty sure it is not possible now without replacing the whole rules array.
Allowing custom modifications is certainly a must have feature, and I think there is already a pull request that does something very similar.
The only difference is that it takes build options as a parameter instead of getting webpack config.
Making it accept both as parameters (cfg, buildOptions) would be the right way to go in my opinion.

This pull request, however, lacks UT tests, E2E tests and documentation. I'd appreciate any contribution, but it would be really great if you contributed to this pull request instead of creating a new one. What do you think?

mistic100 commented 5 years ago

Ok I can start form this PR, but it will be a new one anyway, I can't wait to know if jloscos allows me to access his repo.

just-jeb commented 5 years ago

Huh that's right, I'm just used to having access to all the PRs 😁. Anyway, you got my point, I think that is a common functionality which should be implemented as one piece. Of course, the function in this pull request returns a config that is merged later on to the default config, while what you suggest is rather getting the config and modifying it, but I think it's better to have a single way for doing advanced modifications. Agree?

mistic100 commented 5 years ago

Actually #152 is only about allowing the custom webpack file to export a function.

If I want to be able to alter the final configuration, I need a second file which will be called after "webpack-merge"

mistic100 commented 5 years ago

What do you think about a single file with this structure ?

module.exports = function(buildOptions) {
   return "extra configuration";
};

module.exports.postMerge = function(config, buildOptions) {
  return "full modified configuration";
};
just-jeb commented 5 years ago

I think that it should be as simple as possible. Meaning if the file exports an object then we merge it with default config. If the file exports a function then we pass the default config into this function and expect to get the final configuration in return. Another way is supporting these options independently but I can't think why one would want to keep part of config as an object and apply another part in a function. Am I missing something?

mistic100 commented 5 years ago

Ok so if it is a function there is no automatic merge at all, that's good to me.