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

Shakapacker conflicts with vue-loader #87

Closed
jherdman opened this issue Mar 29, 2022 · 11 comments
Closed

Shakapacker conflicts with vue-loader #87

jherdman opened this issue Mar 29, 2022 · 11 comments
Labels

Comments

@jherdman
Copy link
Contributor

Ruby version: ruby 2.6.6p146 (2020-03-31) [x86_64-darwin17]
Rails version: Rails 6.1.4.1
webpack: 5.70.0
webpack-cli: 4.9.2
webpack-dev-server 4.7.4

Expected behavior

Vue files are correctly handled when using the vue-loader.

Actual behavior

The "raw" rule incorrectly tricks the vue-loader's foo.html.vue check (https://github.com/vuejs/vue-loader/blob/7123a37ff68eaaebec515b029fb602cb4d8cf498/lib/plugin-webpack5.js#L76-L80) causing the vue-loader to believe that it is misconfigured.

The easiest work-around is to add to the exclude configuration of the "raw" rule as follows:

/\.(js|mjs|jsx|ts|tsx|vue\.html)$/

@jherdman jherdman added the bug label Mar 29, 2022
@jherdman
Copy link
Contributor Author

Other options:

  1. Provide instructions on how to override, or exclude, rules provided by shakapacker
  2. Make any, and all, rules opt-in

@tomdracz
Copy link
Collaborator

tomdracz commented Apr 3, 2022

Hey @jherdman Thanks for reporting. I don't know much about vue - what's the difference between .vue.html extension vs .html.vue?

Agree that we need better docs around this, at the moment we mostly got a small paragraph at the end of https://github.com/shakacode/shakapacker#webpack-configuration that touches on this but some better examples would probably be great. I'll try and get this going soon but any contributions are most welcome.

Not sure about makes all rules opt-in, we try and provide fairly "out of the box" config to cover the basic use cases. There are number of permutations here and it's hard to find one-size fits all. The configuration layer is fully optional, if you decide to roll your own webpack config, that might be an option if you're fighting against the defaults.

Best way forward in my head, is making override instructions clearer and perhaps documenting the default behaviour bit better.

Thoughts @justin808 ?

@jherdman
Copy link
Contributor Author

jherdman commented Apr 3, 2022

Unsure! I'm not a Vue expert either. The vue-loader added support for *.vue.html here vuejs/vue-loader#1238.

To me the root issue is that there are a number of rules defined in Shakapacker that are challenging to override (or maybe it's just not clear what the sanctioned way to do this is as a Webpack newb). I think (???) vue-loader is doing the right thing, but maybe Shakapacker is too?

@justin808
Copy link
Member

justin808 commented Apr 4, 2022

Other options:

  1. Provide instructions on how to override, or exclude, rules provided by shakapacker
  2. Make any, and all, rules opt-in
  1. Yes, would be awesome to have better documentation on this.
  2. We've had another request on enabling features not just based on the "magic" of the package is available, but a more direct way. See Easier granular opt-out of defaults #80.

I think for #2, we could have a section in the main webpacker.yml file.

@justin808
Copy link
Member

@jherdman once you get the default configuration, you're free to change it.

See if this example helps:

https://github.com/shakacode/react_on_rails_demo_ssr_hmr/blob/master/config/webpack/commonWebpackConfig.js#L3

Take a look at the base config and see how it gets updated.

@jherdman
Copy link
Contributor Author

jherdman commented Apr 4, 2022

Yeah, we're tweaking the webpack config. The challenge is knowing what's in the default config in the first place. It's kind of tricky to read how Shakapacker is putting the config together. E.g. this isn't very long, but it look quite a while to parse and grok

const getStyleRule = require('../utils/get_style_rule')
const { canProcess } = require('../utils/helpers')
const { additional_paths: includePaths } = require('../config')
module.exports = canProcess('sass-loader', (resolvedPath) =>
getStyleRule(/\.(scss|sass)(\.erb)?$/i, [
{
loader: resolvedPath,
options: {
sassOptions: { includePaths }
}
}
])
)
.

I think my ideal world would be a lot like #80. I'm increasingly convinced that the cardinal sin of webpacker was that it did soooo much, and it wasn't clear what those things were. If one wants to use a complicated tool like Webpack, a shim needs to be both clear, and offer escape valves.

@justin808
Copy link
Member

@tomdracz @Judahmeek, we really need this explicit opt-in or opt-out vs. just the "magic" way that's now done.

@tomdracz
Copy link
Collaborator

@justin808 Not sure about the best way forward here:

We could extend/amend moduleExist/canProcess helpers to take some additional config from webpacker.yml. This would allow for 'opt-out' but I don't think it solves the underlying problem.

Issue is - using webpack-merge for overrides is not trivial. It's all well and good for adding things but it doesn't work too well for modifying and removing rules or plugins. You pretty much have to find the rule, modify it by hand and chuck it back in the config.

I'm wondering whether the rules should be copied over to the project repos to allow easy modification rather than live obscured by Shakapacker. Something conceptually similar to create-react-app eject maybe? https://create-react-app.dev/docs/available-scripts/#npm-run-eject

The above would give us following options:

  • You can use default config with sparse overrides that we would document through some recipes (amending plugin/rule, adding new configs, removing stuff)
  • You can "eject" the configuration and have fine grained access to everything without going through Shakapacker internals
  • Roll your own Webpack config - as long as it spits out manifest, it should work (document how does standalone webpack config look like?)

There's also probably a case there for slimming down what comes in by default or by "magic". Coffee, sass, styles, less could all probably be removed from the defaults. What do we want as a bare minimum?

@Eusebius1920
Copy link

I had similar problems before with webpacker / shakapacker.

Example. I wanted to eject typescript definition files from rails-erb-loader, which is preconfigured in shakapacker. (So that VSCode can read them in and provide autosuggestions and typesafety for .ts.erb files - it is just so comfortable to have that!)

I solved it using a webpack plugin that can inject arbitrary loaders in existing loader-chains based on three configuration options:

  1. moduleRegex: which files are affected (here: /\.ts\.erb$/)
  2. loaderRegex: which loader should we hook in after? (here: /\/rails-erb-loader\//)
  3. loaderPath: Path to the loader that will be injected at that position into the chain (here: a custom loader that just returns the input and writes the typescript definitions to the project folder as a side effect)

The Plugin has only about 30 LoC. But it is so powerful to be able to modifiy exiting chains by adding loaders.

@tomdracz
Copy link
Collaborator

Hey @Eusebius1920 is that plugin something you wrote yourself or open source? Would be great to see that as might be good thing to explore

@Eusebius1920
Copy link

I just keep it in /config/webpack/plugins inside the rails application.

/config/webpack/plugins/chain-loader-injection.js:

const schemaUtils = require("schema-utils");

const pluginName = "ChainLoaderInjection";

const schema = {
  type: "object",
  properties: {
    moduleRegex: {
      description: "specifies the files that are affected by the injection",
      instanceOf: "RegExp",
    },
    loaderRegex: {
      description: "specifies after which loader the injection should happen",
      instanceOf: "RegExp",
    },
    loaderPath: {
      description: "specifies the path to the loader-function that gets injected",
      type: "string",
    },
  },
  additionalProperties: false,
};

class ChainLoaderInjection {
  constructor(options = {}) {
    schemaUtils.validate(schema, options, {
      name: pluginName,
    });

    this.options = options;
  }

  apply(compiler) {
    compiler.hooks.beforeCompile.tap(pluginName, (params) => {
      params.normalModuleFactory.hooks.afterResolve.tap(
        pluginName,
        (resolveData) => {
          if (this.options.moduleRegex.test(resolveData.request)) {
            resolveData.createData.loaders.forEach((loader, index) => {
              if (!this.options.loaderRegex.test(loader.loader)) return;

              resolveData.createData.loaders.splice(index, 0, {
                loader: this.options.loaderPath,
              });
            });
          }
        }
      );
    });
  }
}

module.exports = ChainLoaderInjection;

Usage would then be something like inside /config/webpack/webpack.config.js:

const { webpackConfig, merge } = require("shakapacker");
const path = require("path");

const ChainLoaderInjection = require("./plugins/chain-loader-injection")

module.exports = merge(
  webpackConfig,
  {
    plugins: [
      new ChainLoaderInjection({
        loaderRegex: /\/rails-erb-loader\//,
        moduleRegex: /\.ts\.erb$/,
        loaderPath: path.resolve(
          __dirname,
         "./loaders/my-custom-loader.js"
        )
      })
    ]
  }
);

This could be a starting point for customization of shakapacker's predefined chains. Although I am not 100% sure if this is a good idea.

  • It might lead back to webpacker v5 with too much compilicated wrapping around webpack?
  • And it might not be powerful enough to get the needed level of detail of customizations?

@ahangarha ahangarha added bug low priority Minor issues to fix and removed bug labels Dec 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants