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

Fix build error when the theme changes component style #53

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

soouup
Copy link

@soouup soouup commented Aug 23, 2023

Types of changes

  • New feature
  • Bug fix
  • Documentation change
  • Coding style change
  • Refactoring
  • Performance improvement
  • Test cases
  • Continuous integration
  • Typescript definition change
  • Breaking change

Background and context

It will cause error when building an Arco theme with custom component style.

Solution

The reason for the problem is that the token.less file is not imported by AppendLoader, so I import 'token.less' in each component style files.

How is the change tested?

Just add a example with a arco theme @arco-design/theme-christmas.

Changelog

Changelog(CN) Changelog(EN) Related issues
修复构建带有自定义组件样式的主题会出现报错的问题 Fix build error when the theme changes component style

Checklist:

  • Provide changelog for relevant changes (e.g. bug fixes and new features) if applicable.
  • Changes are submitted to the appropriate branch (e.g. features should be submitted to feature branch and others should be submitted to master branch)

Other information

@Asuka109
Copy link
Contributor

The _addAppendLoader() property is a common util function used by somewhere. I perfer to move it to handleComponentStyle() and only inject tokens for limited files.

handleComponentStyle(compiler: Compiler) {
if (!this.options.theme) return;
const componentList = getThemeComponents(this.options.theme);
componentList.forEach((name) => {
this._addAppendLoader(
compiler,
`**/node_modules/${ARCO_DESIGN_COMPONENT_NAME}/{es,lib}/${name}/style/index.less`,
`${this.options.theme}/components/${name}/index.less`
);
this._addAppendLoader(
compiler,
`**/node_modules/${ARCO_DESIGN_COMPONENT_NAME}/{es,lib}/${name}/style/index.css`,
`${this.options.theme}/components/${name}/index.css`
);
});
}

But when I look back at the existing sourcecode, I recall what make this problem happen: The unplugin turn to inject independent module rules instead of modifing properties of normal modules (it caused really much issues). And so that we deprecated the modifyVars option field:
For maintainability reasons, `@arco-plugins/unplugin-react` no longer supports the `modifyVars` and `varsInjectScope` configuration items. You can achieve the same function by manually configuring the `less-loader`.

It also mean that we doesn't support inject tokens from custom theme implicitly.
/**
* 读取主题变量,添加到 less-loader
*/
hookForVariables() {
const themeVars = this.getThemeVars() || {};
const modifyVars = this.options.modifyVars || {};
const vars = { ...themeVars, ...modifyVars };
if (isEmpty(vars)) return;
let noLessLoaderWarning = '';
const context = getContext(this.compiler, this.options.context);
const include = this.options.varsInjectScope || [];
const isMatch = pathUtils.matcher(include, {
extraGlobPattern: lessMatchers,
extensions: ['.less'],
cwd: context,
});
hookNormalModuleLoader(this.compiler, PLUGIN_NAME, (_loaderContext, module, resource) => {
if (isMatch(resource)) {
const loaders = cloneDeep(module.loaders);
const lessLoader = getLoader(loaders, 'less-loader');
if (!lessLoader) {
noLessLoaderWarning =
'less-loader not found! The theme and modifyVars has no effective, please check if less-loader is added.';
return;
}
rewriteLessLoaderOptions(lessLoader, {
modifyVars: vars,
});
module.loaders = loaders;
}
});
this.compiler.hooks.afterCompile.tap(PLUGIN_NAME, () => {
if (noLessLoaderWarning) {
print.warn(`[arco-design/webpack-plugin]: ${noLessLoaderWarning}`);
}
});
}

However the automatic injection is not a required feature for every theme. I think that theme package should import token definitions by themself. This is also what the default theme of arco do. So that we didn't find the bug out before.

But there is a workaround way. I left a util here:

export function getThemeVars(theme: string) {
try {
const variableLessPath = require.resolve(`${theme}/tokens.less`);
const source = fs.readFileSync(variableLessPath);
return transformSourceToObject(source.toString());
} catch (error) {
return {};
}
}

You can use it just like:

module.exports = {
  module: {
    rules: [
      {
        type: 'css',
        test: /\.less$/,
        use: [
          {
            loader: 'less-loader',
            options: {
              lessOptions: {
                javascriptEnabled: true,
              },
              modifyVars: {
                ...getThemeVars('@arco-design/theme-christmas'),
              }
            },
          },
        ],
      },
    ],
  },
};

You can try it can give some feedback.
Whatmore in my opinion just injecting the token file is a better way. I don't know why the legacy webpack plugin intend to parse the token source file into options of modifyVar. Maybe we need some further disscussion.
@yinkaihui cc~

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

Successfully merging this pull request may close these issues.

2 participants