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

Recompiling with modifyVars #303

Closed
jacobp100 opened this issue Aug 29, 2019 · 8 comments
Closed

Recompiling with modifyVars #303

jacobp100 opened this issue Aug 29, 2019 · 8 comments

Comments

@jacobp100
Copy link

  • Operating System: Windows
  • Node Version: 10.16.0
  • NPM Version: yarn 1.16.0
  • webpack Version: 4.39.2
  • less-loader Version: 5.0.0

Expected Behavior / Situation

Use modify vars, like the following

{
  loader: 'less-loader',
  options: {
    modifyVars: require("path/to/theme.js")
  }
}

When in watch mode, any changes to theme.js will not be picked up, and you have to restart the dev server

Actual Behavior / Situation

Actual behaviour is the expected behaviour

Modification Proposal

If we allowed the user to specify a path to modifyVars,

 {
   loader: 'less-loader',
   options: {
-    modifyVars: require("path/to/theme.js")
+    modifyVars: require.resolve("path/to/theme.js")
   }
 }

We can then modify getOptions.js with the following,

function getOptions(loaderContext) {
  ...
  if (typeof options.modifyVars === "string") {
    const file = options.modifyVars;
    loaderContext.addDependency(file); // *
    require.cache[file] = undefined;
    options.modifyVars = require(file);
  }
  ...
}

(*) Note: I'm fully sure if there's side effects to adding a dependency outside of watch mode. I'm a bit worried it might sneak this file into some bundled JS output in some cases (although it doesn't seem to actually do this for me)

@alexander-akait
Copy link
Member

Sorry, it is limitation, we want to remove modifyVars in next major release, so i recommended to restructure your app

@jacobp100
Copy link
Author

Ah - good to know. I'm using ant design, and modifyVars is their recommendation, but I'm sure there'll be another way to do the same thing!

Is the 'we' wanting to remove it the webpack loader, or less in general?

@alexander-akait
Copy link
Member

it is less-loader feature

@jacobp100
Copy link
Author

Are we definitely talking about the same feature? 😅

It’s on the less docs under the ‘modify variables’ section. http://lesscss.org/usage/#less-options

@alexander-akait
Copy link
Member

Oh, my mistake

@cap-Bernardito
Copy link
Member

modifyVars was removed in favor prependData/appendData

The scheme for solving your problem:
theme.less

@color1: coral;

webpack.config.js

...
{
    loader: "less-loader",
    options: {
        appendData: (loader) => {
            loader.addDependency(
                path.resolve(__dirname, "path/to/theme.less")
            );

            return fs.readFileSync(
                path.resolve(__dirname, "path/to/theme.less")
            ).toString();
        },
    },
...

@hooper-hc
Copy link

modifyVars was removed in favor prependData/appendData

The scheme for solving your problem:
theme.less

@color1: coral;

webpack.config.js

...
{
    loader: "less-loader",
    options: {
        appendData: (loader) => {
            loader.addDependency(
                path.resolve(__dirname, "path/to/theme.less")
            );

            return fs.readFileSync(
                path.resolve(__dirname, "path/to/theme.less")
            ).toString();
        },
    },
...

This is a bad change, next

@kagawagao
Copy link

kagawagao commented May 18, 2020

modifyVars was removed in favor prependData/appendData
The scheme for solving your problem:
theme.less

@color1: coral;

webpack.config.js

...
{
    loader: "less-loader",
    options: {
        appendData: (loader) => {
            loader.addDependency(
                path.resolve(__dirname, "path/to/theme.less")
            );

            return fs.readFileSync(
                path.resolve(__dirname, "path/to/theme.less")
            ).toString();
        },
    },
...

This is a bad change, next

@hooper-hc you can still use modifyVars, but put it into lessOptions

options: {
  lessOptions: {
    modifyVars: {yourVar: 'xxx'}
  }
}

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

No branches or pull requests

5 participants