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(commitlint): 🐛 repair commitlint configuration failures #103

Merged
merged 3 commits into from
Mar 25, 2021

Conversation

yi-Xu-0100
Copy link
Collaborator

@yi-Xu-0100 yi-Xu-0100 commented Mar 25, 2021

After webpack, it needs fix the require expression when require dynamic resources.

close #102

After webpack, it need fix the require expression.

close #102
@yi-Xu-0100 yi-Xu-0100 requested a review from vivaxy March 25, 2021 01:23
@yi-Xu-0100 yi-Xu-0100 added the wait-to-build PR wait to build package. label Mar 25, 2021
@github-actions
Copy link
Contributor

The beta extension of this pull has been built in the workflow! 🎉
Download vscode-conventional-commits-84beb69.vsix in artifacts. 🚀

@github-actions github-actions bot removed the wait-to-build PR wait to build package. label Mar 25, 2021
@yi-Xu-0100 yi-Xu-0100 added the wait-to-build PR wait to build package. label Mar 25, 2021
@github-actions
Copy link
Contributor

The beta extension of this pull has been built in the workflow! 🎉
Download vscode-conventional-commits-fbf399f.vsix in artifacts. 🚀

@github-actions github-actions bot removed the wait-to-build PR wait to build package. label Mar 25, 2021
@vivaxy
Copy link
Owner

vivaxy commented Mar 25, 2021

Are we rewriting @commitlint/load, resolve-extends, import-fresh and resolve-global?
It there another robust way to do it? What if there is another module with require installed?
Can we make change to webpack to fix this?

@yi-Xu-0100
Copy link
Collaborator Author

@vivaxy In the most time, The require will lead the webpack to bundle the JS into extension.js. But sometime, we need to require the dynamic resources, and the webpack will show warning about it. 😀

webpack 5.25.0 compiled with 9 warnings in 6736 ms

** if we make stat.warnings = true in webpack.config.js, the warning will show as follow. **

WARNING in ./node_modules/@commitlint/load/lib/load.js 34:24-53
Critical dependency: the request of a dependency is an expression
 @ ./src/lib/commitlint.ts 16:15-42
 @ ./src/lib/conventional-commits.ts 21:21-44
 @ ./src/extension.ts 9:31-68

WARNING in ./node_modules/@commitlint/load/lib/utils/load-plugin.js 22:21-38
Critical dependency: the request of a dependency is an expression
 @ ./node_modules/@commitlint/load/lib/load.js 14:38-68
 @ ./src/lib/commitlint.ts 16:15-42
 @ ./src/lib/conventional-commits.ts 21:21-44
 @ ./src/extension.ts 9:31-68

WARNING in ./node_modules/@commitlint/load/lib/utils/load-plugin.js 27:16-41
Critical dependency: the request of a dependency is an expression
 @ ./node_modules/@commitlint/load/lib/load.js 14:38-68
 @ ./src/lib/commitlint.ts 16:15-42
 @ ./src/lib/conventional-commits.ts 21:21-44
 @ ./src/extension.ts 9:31-68

WARNING in ./node_modules/@commitlint/load/lib/utils/load-plugin.js 42:33-58
Critical dependency: the request of a dependency is an expression
 @ ./node_modules/@commitlint/load/lib/load.js 14:38-68
 @ ./src/lib/commitlint.ts 16:15-42
 @ ./src/lib/conventional-commits.ts 21:21-44
 @ ./src/extension.ts 9:31-68

WARNING in ./node_modules/@commitlint/resolve-extends/lib/index.js 40:40-47
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted
 @ ./node_modules/@commitlint/load/lib/load.js 13:42-80
 @ ./src/lib/commitlint.ts 16:15-42
 @ ./src/lib/conventional-commits.ts 21:21-44
 @ ./src/extension.ts 9:31-68

WARNING in ./node_modules/@commitlint/resolve-extends/lib/index.js 55:28-57
Critical dependency: the request of a dependency is an expression
 @ ./node_modules/@commitlint/load/lib/load.js 13:42-80
 @ ./src/lib/commitlint.ts 16:15-42
 @ ./src/lib/conventional-commits.ts 21:21-44
 @ ./src/extension.ts 9:31-68

WARNING in ./node_modules/import-fresh/index.js 31:31-48
Critical dependency: the request of a dependency is an expression
 @ ./node_modules/@commitlint/resolve-extends/lib/index.js 22:20-43
 @ ./node_modules/@commitlint/load/lib/load.js 13:42-80
 @ ./src/lib/commitlint.ts 16:15-42
 @ ./src/lib/conventional-commits.ts 21:21-44
 @ ./src/extension.ts 9:31-68

WARNING in ./node_modules/resolve-global/index.js 7:9-71
Critical dependency: the request of a dependency is an expression
 @ ./node_modules/@commitlint/resolve-extends/lib/index.js 18:0-25
 @ ./node_modules/@commitlint/load/lib/load.js 13:42-80
 @ ./src/lib/commitlint.ts 16:15-42
 @ ./src/lib/conventional-commits.ts 21:21-44
 @ ./src/extension.ts 9:31-68

WARNING in ./node_modules/resolve-global/index.js 9:9-70
Critical dependency: the request of a dependency is an expression
 @ ./node_modules/@commitlint/resolve-extends/lib/index.js 18:0-25
 @ ./node_modules/@commitlint/load/lib/load.js 13:42-80
 @ ./src/lib/commitlint.ts 16:15-42
 @ ./src/lib/conventional-commits.ts 21:21-44
 @ ./src/extension.ts 9:31-68

Before this version, when we make a build it will occur 9 warning and I do not know the solution and what will cause. I try to figure out the information about Critical dependency, the solution is to make it silent, and I do it also. 😥

After the thinking, the people marked the warning as silent because they have fully knowledge about the result. So I delete the stat.warning and will fix every warning next time. 😀

I think the most time we do not need the dynamic resources from the environment. So it is not be worried about every require expression.

I did not find the more elegant way to fix the issue. It can be upgrade if there are any suggestions. 😊

@vivaxy
Copy link
Owner

vivaxy commented Mar 25, 2021

@yi-Xu-0100 Thank you for your explanation.

Is it possible to mark the Critical dependency: the request of a dependency is an expression as an error, so we can notice the problem while building.

@yi-Xu-0100
Copy link
Collaborator Author

yi-Xu-0100 commented Mar 25, 2021

@vivaxy If you test the beta version with commitlint and it works well, I think we should make it as a patch as soon as possible. 😀

If I found a better way to upgrade it, I will try to make a PR. 😜
I don't find a way to mark the Critical dependency: the request of a dependency is an expression as an error, I will progress on it in the future.

By changing warnings to errors, webpack can recognize every warning as errors.
@yi-Xu-0100 yi-Xu-0100 added the wait-to-build PR wait to build package. label Mar 25, 2021
@github-actions
Copy link
Contributor

The beta extension of this pull has been built in the workflow! 🎉
Download vscode-conventional-commits-9ecd175.vsix in artifacts. 🚀

@github-actions github-actions bot removed the wait-to-build PR wait to build package. label Mar 25, 2021
@yi-Xu-0100
Copy link
Collaborator Author

@vivaxy I find a plugin of webpack to change warnings to errors. 🤪

I think the warning also can solve by the webpack plugin, but I didn't find it. 😅

I find a similar post about it. But the code is old for the webpack 5. And the corresponding codes has been deleted in the latest version. 😥

@yi-Xu-0100 yi-Xu-0100 merged commit b79bb31 into master Mar 25, 2021
@yi-Xu-0100 yi-Xu-0100 deleted the fix-commitlint-configuration-failure branch March 25, 2021 07:22
@vivaxy
Copy link
Owner

vivaxy commented Mar 25, 2021

@vivaxy I find a plugin of webpack to change warnings to errors. 🤪

I think the warning also can solve by the webpack plugin, but I didn't find it. 😅

I find a similar post about it. But the code is old for the webpack 5. And the corresponding codes has been deleted in the latest version. 😥

Add more refs:

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.

[BUG] Extenstion stopped recognizing commitlint rules
2 participants