-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
simplify webpack config #330
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
Warning Rate limit exceeded@ryoppippi has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 25 minutes and 2 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes involve a significant restructuring of the Webpack configuration for the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
7d11f58
to
781ddc2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Inline review comments failed to post
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- examples/webpack-minimal/webpack.config.ts (1 hunks)
- packages/unplugin-typia/README.md (1 hunks)
- packages/unplugin-typia/src/webpack.ts (2 hunks)
Additional context used
LanguageTool
packages/unplugin-typia/README.md
[style] ~201-~201: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...se 'cjs', use dynamic import. > If you want to use 'esm' target, don't worry! You can ...(REP_WANT_TO_VB)
Markdownlint
packages/unplugin-typia/README.md
208-208: Column: 1
Hard tabs(MD010, no-hard-tabs)
209-209: Column: 1
Hard tabs(MD010, no-hard-tabs)
210-210: Column: 1
Hard tabs(MD010, no-hard-tabs)
211-211: Column: 1
Hard tabs(MD010, no-hard-tabs)
212-212: Column: 1
Hard tabs(MD010, no-hard-tabs)
213-213: Column: 1
Hard tabs(MD010, no-hard-tabs)
214-214: Column: 1
Hard tabs(MD010, no-hard-tabs)
215-215: Column: 1
Hard tabs(MD010, no-hard-tabs)
200-200: null
Blank line inside blockquote(MD028, no-blanks-blockquote)
Additional comments not posted (9)
packages/unplugin-typia/src/webpack.ts (2)
14-14
: Simplification of usage instructions for CommonJS modulesThe change to recommend using a dynamic import for CommonJS usage, regardless of the Node.js version, is a welcome simplification. It provides a consistent and modern approach for handling module imports, improving clarity and reducing confusion for users of the plugin.
23-32
: Modernisation of example Webpack configurationThe updates to the example Webpack configuration, including the use of a dynamic import for the
UnpluginTypia
module and exporting an asynchronous function, are excellent improvements. These changes align the configuration with modern JavaScript practices, allowing for dynamic imports and conditional inclusion of plugins based on the environment. The enhanced compatibility and clarity of the example will benefit users of the plugin.examples/webpack-minimal/webpack.config.ts (5)
7-7
: LGTM!Using the
tsImport
function for dynamic imports is a good practice. It can improve the loading behaviour of the plugin.
11-12
: LGTM!Exporting an asynchronous function is a good approach for enabling dynamic imports and conditional inclusion of plugins. It enhances the flexibility and maintainability of the Webpack configuration.
14-14
: LGTM!Setting the Webpack mode based on the
NODE_ENV
environment variable is a good practice. It allows for different optimisations and behaviours in development and production environments.
27-28
: LGTM!Including the
UnpluginTypia
plugin is essential for the functionality of theunplugin-typia
package. Conditionally including theWorkboxWebpackPlugin.GenerateSW()
plugin based on the environment is a good approach for streamlining the plugin management.
51-51
: LGTM!Using a type assertion with
as const satisfies
is a good practice for ensuring type safety. It confirms that the returned configuration object adheres to the expectedConfiguration
type.packages/unplugin-typia/README.md (2)
199-200
: Looks good!The note provides clear guidance on using dynamic imports for the CommonJS target, which aligns with the objective of simplifying the setup instructions.
Tools
Markdownlint
200-200: null
Blank line inside blockquote(MD028, no-blanks-blockquote)
207-215
: Great update to the Webpack configuration example!The async function and dynamic import of the plugin provide a clearer demonstration of how to use the plugin with Webpack.
Tools
Markdownlint
208-208: Column: 1
Hard tabs(MD010, no-hard-tabs)
209-209: Column: 1
Hard tabs(MD010, no-hard-tabs)
210-210: Column: 1
Hard tabs(MD010, no-hard-tabs)
211-211: Column: 1
Hard tabs(MD010, no-hard-tabs)
212-212: Column: 1
Hard tabs(MD010, no-hard-tabs)
213-213: Column: 1
Hard tabs(MD010, no-hard-tabs)
214-214: Column: 1
Hard tabs(MD010, no-hard-tabs)
215-215: Column: 1
Hard tabs(MD010, no-hard-tabs)
Comments failed to post (2)
packages/unplugin-typia/README.md (2)
200-200: Remove the blank line inside the blockquote.
The blank line inside the blockquote violates the Markdownlint rule MD028. Consider removing the blank line to improve the Markdown formatting.
Apply this diff to remove the blank line:
-
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Tools
Markdownlint
200-200: null
Blank line inside blockquote(MD028, no-blanks-blockquote)
208-215: Use consistent indentation in the code block.
The code block uses hard tabs for indentation, which is inconsistent with the rest of the file. Consider replacing the hard tabs with spaces to improve readability and adhere to the Markdownlint rules.
Apply this diff to replace hard tabs with spaces:
- const { default: UnpluginTypia } = await import('@ryoppippi/unplugin-typia/webpack'); - return { - plugins: [ - new UnpluginTypia({ - // options - }), - ], - }; + const { default: UnpluginTypia } = await import('@ryoppippi/unplugin-typia/webpack'); + return { + plugins: [ + new UnpluginTypia({ + // options + }), + ], + };Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const { default: UnpluginTypia } = await import('@ryoppippi/unplugin-typia/webpack'); return { plugins: [ new UnpluginTypia({ // options }), ], };
Tools
Markdownlint
208-208: Column: 1
Hard tabs(MD010, no-hard-tabs)
209-209: Column: 1
Hard tabs(MD010, no-hard-tabs)
210-210: Column: 1
Hard tabs(MD010, no-hard-tabs)
211-211: Column: 1
Hard tabs(MD010, no-hard-tabs)
212-212: Column: 1
Hard tabs(MD010, no-hard-tabs)
213-213: Column: 1
Hard tabs(MD010, no-hard-tabs)
214-214: Column: 1
Hard tabs(MD010, no-hard-tabs)
215-215: Column: 1
Hard tabs(MD010, no-hard-tabs)
781ddc2
to
34d1770
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (3)
packages/unplugin-typia/README.md (3)
201-201
: Consider rephrasing to avoid repetition.The guidance for the ESM target is clear and aligns with the PR objective. However, the phrasing "If you want to use..." is repetitive with the previous sentence. Consider rephrasing it to add variety, such as:
For the 'esm' target, no additional setup is required. You can use this plugin as is.
Tools
LanguageTool
[style] ~201-~201: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...se 'cjs', use dynamic import. > If you want to use 'esm' target, don't worry! You can ...(REP_WANT_TO_VB)
206-214
: Example configuration looks good, but please fix the formatting.The example webpack configuration using dynamic imports for the plugin aligns well with the PR objective of simplifying the setup.
However, please replace the hard tabs with spaces for consistent formatting across the codebase. Most of the other code blocks in the README use spaces for indentation.
Tools
Markdownlint
207-207: Column: 1
Hard tabs(MD010, no-hard-tabs)
208-208: Column: 1
Hard tabs(MD010, no-hard-tabs)
209-209: Column: 1
Hard tabs(MD010, no-hard-tabs)
210-210: Column: 1
Hard tabs(MD010, no-hard-tabs)
211-211: Column: 1
Hard tabs(MD010, no-hard-tabs)
212-212: Column: 1
Hard tabs(MD010, no-hard-tabs)
213-213: Column: 1
Hard tabs(MD010, no-hard-tabs)
214-214: Column: 1
Hard tabs(MD010, no-hard-tabs)
200-200
: Please remove the blank line inside the blockquote.To improve the Markdown formatting consistency, please remove the blank line inside the blockquote. Blank lines are generally not recommended within blockquotes in Markdown.
Tools
Markdownlint
200-200: null
Blank line inside blockquote(MD028, no-blanks-blockquote)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/unplugin-typia/README.md (1 hunks)
- packages/unplugin-typia/src/webpack.ts (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/unplugin-typia/src/webpack.ts
Additional context used
LanguageTool
packages/unplugin-typia/README.md
[style] ~201-~201: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...se 'cjs', use dynamic import. > If you want to use 'esm' target, don't worry! You can ...(REP_WANT_TO_VB)
Markdownlint
packages/unplugin-typia/README.md
207-207: Column: 1
Hard tabs(MD010, no-hard-tabs)
208-208: Column: 1
Hard tabs(MD010, no-hard-tabs)
209-209: Column: 1
Hard tabs(MD010, no-hard-tabs)
210-210: Column: 1
Hard tabs(MD010, no-hard-tabs)
211-211: Column: 1
Hard tabs(MD010, no-hard-tabs)
212-212: Column: 1
Hard tabs(MD010, no-hard-tabs)
213-213: Column: 1
Hard tabs(MD010, no-hard-tabs)
214-214: Column: 1
Hard tabs(MD010, no-hard-tabs)
200-200: null
Blank line inside blockquote(MD028, no-blanks-blockquote)
Additional comments not posted (1)
packages/unplugin-typia/README.md (1)
199-199
: Looks good!The guidance to use dynamic imports for the CommonJS target is clear and aligns with the PR objective of simplifying the webpack configuration.
@sacru2red FYI |
I got to know that we don't need
jiti
to import ESM plugin for webpack configuration.Ref Docs: https://unocss.dev/integrations/webpack
Related Issue: samchon/typia#1094
Summary by CodeRabbit
New Features
unplugin-typia
plugin, promoting dynamic imports for CommonJS module systems.Documentation
unplugin-typia
plugin, removing unnecessary complexity related to Node.js versions.