Stable/v0 0 1 #35
Stable/v0 0 1 #35
Conversation
…ad handling This commit includes a number of changes in the console involving new classes and the removal of old ones all aimed at improving thread handling in the console. It introduces new classes like `ConsoleThreadSend`, `ConsoleBuildBase`, `ConsoleThreadFormat`, `ConsoleWatchBase`, `ConsoleThreadReceive` and removes old ones like `thread-console.ts` and `watch-console.ts`. These changes make the console output thread aware, ensuring messages from different threads are outputted correctly, which markedly improves the clarity and orderliness of the console output.
The code changes improved the behavior and error handling of Typescript and Rollup build tasks within the CLI. Notably, the handling of dependencies in multi-threaded builds was advanced. Unnecessary/unused import was also removed in "common.ts". Other updates include improvements to console logging and error handling for clarity and better debugging. The 'zTSPreDiagnostics' function was further refactored for improved accuracy.
The commit includes a new plugin that enforces no relative import in the Zenflux Eslint configuration. This plugin encourages more consistent and maintainable code by restricting the use of local file paths in import statements. Import statements using relative paths are flagged as errors, and a message is provided to guide users towards the preferred path alias syntax.
…guides Adjusted some TypeScript rules: changed `typescript_import_prefer_absolute_path` to false and `typescript_import_sort_module_name` to true, as well as updated `typescript_import_use_node_resolution` to global. Also, `typescript_use_import_type` is now always used. Furthermore, whitespace in `ij_visual_guides` and `ij_property_prefix` for both JavaScript and TypeScript has been removed for cleaner code.
Updated dependencies for several packages including react-x-env and react-noop-renderer to include '@zenflux/react-scheduler'. Removed 'bun-types' from the types array in the zenflux-cli tsconfig.json. Modified scripts in the main package.json including updating the version and packageManager properties, and adding a new "@z-flux:dev" command.
This workflow is responsible for building and running tests for React packages. It runs on each push and pull request, checking out the code, setting up the environment, installing dependencies, building the code, and finally running the tests.
iNewLegend
force-pushed
the
stable/v0-0-1
branch
2 times, most recently
from
05a07f0
to
179e397
Compare
Updates the testing environment for react packages in GitHub Actions, allowing tests to run either using GitHub's host or self-hosted runners.
iNewLegend
force-pushed
the
stable/v0-0-1
branch
18 times, most recently
from
08e07fe
to
66c6cf0
Compare
|
/describe |
|
/describe |
PR Review 🔍
Code feedback:
|
…ptions to `readme.md` This update adds two new options to the command build base: `--no-diagnostic`, which disables typescript diagnostics, and `--no-declaration`, which disables declaration file generation and stops using the api-exporter. Both options and their behaviors have also been documented in the README file.
iNewLegend
force-pushed
the
stable/v0-0-1
branch
2 times, most recently
from
ff58c12
to
cbd94d0
Compare
…ames in workflows The environment variable names in workflows `1-main-github-hosted.yml` and `2-manage-runners.yml` have been updated to be more explicit about their purpose. This improves readability and maintainability of the code. The logic remains the same, only the variable names have been changed to reflect their functionalities better.
…console-thread-format.ts` Detailed docs were added to the `ConsoleThreadFormat` class in `console-thread-format.ts`. They provide information on the expected output of several abstract methods, the class's purpose, and the behavior of the constructor function. This enhances code clarity and promotes proper implementation of the class methods.
This commit adds valuable explanatory comment describing the purpose and function of the `ConsoleThreadSend` class in `console-thread-send.ts`. This class manages sending operations in a multithreaded console environment, using the functionalities provided by ConsoleManager.
| class SimpleOutputStrategy implements OutputStrategy { | ||
| private $: ConsoleWatchBase; | ||
|
|
||
| public constructor( base: ConsoleWatchBase ) { | ||
| this.$ = base; | ||
| } | ||
|
|
||
| public output( method: TConsoleLoggerMethod, args: any[] ) { | ||
| this.$.getLogWidget().log( args.join( " " ) ); | ||
| } | ||
| } | ||
|
|
||
| class DebugOutputStrategy implements OutputStrategy { | ||
| private $: ConsoleWatchBase; | ||
|
|
||
| public constructor( base: ConsoleWatchBase ) { | ||
| this.$ = base; | ||
| } | ||
|
|
||
| public output( method: TConsoleLoggerMethod, args: any[] ) { | ||
| if ( method.name === this.$.debug.name ) { | ||
|
|
||
| logWidgetDebugBuffer.push( args.join( " " ) ); | ||
|
|
||
| return this.addDebouncedLog(); | ||
| } | ||
|
|
||
| this.$.getLogWidget().log( args.join( " " ) ); | ||
| } | ||
|
|
||
| private addDebouncedLog() { | ||
| // Use debounce to prevent too many logs. | ||
| if ( logWidgetDebugBuffer.length > LOG_MAX_BUFFER_SIZE ) { | ||
| this.createLogAndClearBuffer(); | ||
| } | ||
|
|
||
| zDebounce( `console-${ this.$.getName() }-debug-buffer`, () => { | ||
| if ( logWidgetDebugBuffer.length ) { | ||
| this.createLogAndClearBuffer(); | ||
| } | ||
| }, LOG_DEBUG_DEBOUNCE_DELAY ); | ||
| } | ||
|
|
||
| private createLogAndClearBuffer() { | ||
| logWidgetDebug!.log( logWidgetDebugBuffer.join( "\n" ) ); | ||
| logWidgetDebugBuffer.length = 0; | ||
| } | ||
| } |
There was a problem hiding this comment.
/ask Can you explain what is the difference between the strategies?
There was a problem hiding this comment.
The SimpleOutputStrategy and DebugOutputStrategy classes implement the OutputStrategy interface but differ in how they handle logging:
SimpleOutputStrategy
- Purpose: Directly logs the provided arguments.
- Method:
output(method: TConsoleLoggerMethod, args: any[]): Joins theargsarray into a single string and logs it using thelogmethod of the log widget obtained fromthis.$.getLogWidget().
DebugOutputStrategy
- Purpose: Adds a layer of buffering and debouncing for debug logs to prevent excessive logging.
- Methods:
output(method: TConsoleLoggerMethod, args: any[]):- Checks if the method name matches
this.$.debug.name. - If it matches, it pushes the joined
argsintologWidgetDebugBufferand callsaddDebouncedLog(). - If it doesn't match, it logs the arguments directly like
SimpleOutputStrategy.
- Checks if the method name matches
addDebouncedLog():- Uses a debounce mechanism to limit the frequency of log updates.
- If the buffer exceeds
LOG_MAX_BUFFER_SIZE, it callscreateLogAndClearBuffer(). - Uses
zDebounceto schedule the buffer to be logged and cleared afterLOG_DEBUG_DEBOUNCE_DELAY.
createLogAndClearBuffer(): Joins the buffer into a single string, logs it usinglogWidgetDebug, and then clears the buffer.
Summary
SimpleOutputStrategy: Direct logging without any additional logic.DebugOutputStrategy: Buffered and debounced logging specifically for debug messages to manage log volume and frequency.
There was a problem hiding this comment.
/ask can you suggest /** comment **/ to explain it?
There was a problem hiding this comment.
Certainly! Here are suggested comments to explain the SimpleOutputStrategy and DebugOutputStrategy classes:
/**
* SimpleOutputStrategy class implements the OutputStrategy interface.
* It provides a simple logging mechanism by directly logging the provided arguments.
*/
class SimpleOutputStrategy implements OutputStrategy {
private $: ConsoleWatchBase;
/**
* Constructor for SimpleOutputStrategy.
* @param base - An instance of ConsoleWatchBase.
*/
public constructor( base: ConsoleWatchBase ) {
this.$ = base;
}
/**
* Outputs the log by joining the arguments and logging them to the log widget.
* @param method - The logging method.
* @param args - The arguments to log.
*/
public output( method: TConsoleLoggerMethod, args: any[] ) {
this.$.getLogWidget().log( args.join( " " ) );
}
}
/**
* DebugOutputStrategy class implements the OutputStrategy interface.
* It provides a logging mechanism with a debounce feature to prevent too many logs.
*/
class DebugOutputStrategy implements OutputStrategy {
private $: ConsoleWatchBase;
/**
* Constructor for DebugOutputStrategy.
* @param base - An instance of ConsoleWatchBase.
*/
public constructor( base: ConsoleWatchBase ) {
this.$ = base;
}
/**
* Outputs the log with a debounce mechanism if the method is debug.
* Otherwise, it logs the arguments directly.
* @param method - The logging method.
* @param args - The arguments to log.
*/
public output( method: TConsoleLoggerMethod, args: any[] ) {
if ( method.name === this.$.debug.name ) {
logWidgetDebugBuffer.push( args.join( " " ) );
return this.addDebouncedLog();
}
this.$.getLogWidget().log( args.join( " " ) );
}
/**
* Adds a debounced log to prevent too many logs.
* If the buffer exceeds the maximum size, it creates a log and clears the buffer.
*/
private addDebouncedLog() {
if ( logWidgetDebugBuffer.length > LOG_MAX_BUFFER_SIZE ) {
this.createLogAndClearBuffer();
}
zDebounce( `console-${ this.$.getName() }-debug-buffer`, () => {
if ( logWidgetDebugBuffer.length ) {
this.createLogAndClearBuffer();
}
}, LOG_DEBUG_DEBOUNCE_DELAY );
}
/**
* Creates a log from the buffer and clears the buffer.
*/
private createLogAndClearBuffer() {
logWidgetDebug!.log( logWidgetDebugBuffer.join( "\n" ) );
logWidgetDebugBuffer.length = 0;
}
}These comments provide a clear explanation of the purpose and functionality of each class and method.
…watch.ts` This new class is responsible for managing the output of debug log messages in a batch to prevent overwhelming logs. The log buffer flushes when the maximum buffer size is attained or after a certain delay period. This strategy is employed when the debug flag is enabled.
Ask❓can you create new issue for this todo? Answer:Certainly! Here is the information extracted from the PR for creating a new issue: Issue Title:Avoid using useless imports from thread, use dynamic imports instead Description:There is a TODO comment in the file File and Line Number:
Code Snippet:/**
* @TODO: Avoid using useless imports from thread, use dynamic imports instead.
*/Suggested Steps:
Would you like me to include any additional details or context in the issue? |
| @@ -1,5 +1,7 @@ | |||
| /** | |||
| * @author Leonid Vinikov <leonidvinikov@gmail.com> | |||
| * | |||
| * @TODO: Avoid using useless imports from thread, use dynamic imports instead. | |||
There was a problem hiding this comment.
/ask can you create new issue for this todo?
There was a problem hiding this comment.
Certainly! Here is a suggested issue description for the TODO:
Title: Refactor Imports to Use Dynamic Imports
Description:
In the file packages/zenflux-cli/src/core/build.ts, there is a TODO comment indicating that we should avoid using useless imports from the thread and instead use dynamic imports. This will help in optimizing the code and potentially improve performance.
TODO Comment:
/**
* @TODO: Avoid using useless imports from thread, use dynamic imports instead.
*/Tasks:
- Identify and remove any unnecessary static imports from the thread.
- Implement dynamic imports where applicable.
- Test to ensure that the functionality remains intact after the changes.
File:
packages/zenflux-cli/src/core/build.ts
References:
PR: 'Stable/v0 0 1'
Branch: 'stable/v0-0-1'
Feel free to adjust the description as needed.
|
|
||
| host.sendMessage( "message", "build", "prepare", | ||
| config.outputName, | ||
| // TODO: Sending formated message to the host, not the best practice. |
There was a problem hiding this comment.
/ask can you create new issue for this todo?
There was a problem hiding this comment.
Certainly! Here is a draft for the new issue based on the TODO comment:
Title: Refactor message sending to host in zRollupBuildInWorker
Description:
The current implementation in zRollupBuildInWorker sends a formatted message to the host, which is not considered best practice. This needs to be refactored to improve code quality and maintainability.
File:
packages/zenflux-cli/src/core/build.ts
Current Code:
// TODO: Sending formated message to the host, not the best practice.
host.sendMessage("message", "build", "prepare", config.outputName);Suggested Improvement:
- Refactor the code to use a more appropriate method for sending messages to the host.
- Consider using
sendVerboseorsendMessagewith a different approach that does not involve passing formatted messages directly.
Additional Context:
This change is aimed at improving the clarity and maintainability of the code. The current approach of sending formatted messages directly is not ideal and should be revised.
Feel free to adjust the issue details as needed!
| } | ||
| } ); | ||
|
|
||
| // TODO: It should favor skipping external dependencies. |
There was a problem hiding this comment.
/ask can you create new issue for this todo?
There was a problem hiding this comment.
Certainly! Here is a suggested issue description for the TODO comment:
Title: Skip External Dependencies in zBuildThreadWaitForDependencies
Description:
In the zBuildThreadWaitForDependencies function within packages/zenflux-cli/src/core/build.ts, there is a TODO comment indicating that the function should favor skipping external dependencies. This needs to be implemented to improve the build process.
File: packages/zenflux-cli/src/core/build.ts
Line: 231 (approximate, based on the provided diff)
Current TODO Comment:
// TODO: It should favor skipping external dependencies.Tasks:
- Analyze the current handling of dependencies in the
zBuildThreadWaitForDependenciesfunction. - Implement logic to skip external dependencies.
- Add appropriate tests to ensure the new logic works as expected.
- Update documentation if necessary.
Additional Context:
The function currently processes dependencies from options.otherConfigs and checks them against packagesDependencies. The new logic should identify external dependencies and skip them during this process.
Feel free to adjust the issue description as needed.
| import fs from "node:fs"; | ||
| import path from "node:path"; | ||
|
|
||
| // TODO: Use cache |
There was a problem hiding this comment.
/ask create issue for this
There was a problem hiding this comment.
To create an issue for the // TODO: Use cache comment, you can follow these steps:
-
Title: Implement Caching in
plugin.js -
Description:
- Context: In the
packages/zenflux-eslint/plugin.jsfile, there is a TODO comment indicating the need to use caching. - Details: The current implementation does not utilize caching, which could potentially improve performance and efficiency.
- Location: The TODO comment is located at the beginning of the
plugin.jsfile. - Task: Implement a caching mechanism in the
plugin.jsfile to optimize performance.
- Context: In the
-
Labels: Enhancement, Performance, TODO
-
Assignee: (Assign to the relevant developer or team)
By creating this issue, it will be tracked and prioritized accordingly.
| fs.readFileSync( globalThis.__Z_ESLINT_CONFIG__.zRootPackagePath, 'utf8' ) | ||
| ); | ||
|
|
||
| // TODO Move out of "@zenflux/eslint" package |
There was a problem hiding this comment.
/ask create issue for this
There was a problem hiding this comment.
To create an issue for the TODO comment, you can use the following template:
Title: Move code out of "@zenflux/eslint" package
Description:
There is a TODO comment in the plugin.js file indicating that some code needs to be moved out of the @zenflux/eslint package. This task involves identifying the specific code that should be relocated and determining the appropriate new location for it.
File: packages/zenflux-eslint/plugin.js
Line: 12
Current TODO Comment:
// TODO Move out of "@zenflux/eslint" package
Tasks:
- Identify the code that needs to be moved.
- Determine the appropriate new location for the code.
- Update the codebase to reflect this change.
- Ensure all references to the moved code are updated accordingly.
- Test to ensure no functionality is broken due to this change.
Priority: Medium
Labels: refactoring, enhancement
This issue will help track the progress of this TODO item and ensure it gets addressed in a timely manner.
| import type { | ||
| TZCreateDeclarationWorkerOptions, | ||
| TZPreDiagnosticsOptions, | ||
| TZPreDiagnosticsWorkerOptions | ||
| } from "@zenflux/cli/src/definitions/typescript"; | ||
|
|
||
| import type { Thread, ThreadHost } from "@zenflux/cli/src/modules/threading/thread"; | ||
| import type { Worker } from "@zenflux/cli/src/modules/threading/worker"; | ||
| import type { IZConfigInternal } from "@zenflux/cli/src/definitions/config"; | ||
|
|
||
| // TODO: Avoid this, create threadPool with max threads = cpu cores. |
There was a problem hiding this comment.
/ask Create issue from todo
There was a problem hiding this comment.
To create an issue from the TODO comment, you can use the following details:
Title: Create threadPool with max threads equal to CPU cores
Description:
There is a TODO comment in the packages/zenflux-cli/src/core/typescript.ts file that suggests creating a thread pool with a maximum number of threads equal to the number of CPU cores. This enhancement aims to optimize the threading model by avoiding the creation of an excessive number of threads, which can lead to performance degradation.
Location in Code:
// TODO: Avoid this, create threadPool with max threads = cpu cores.Proposed Solution:
Implement a thread pool that dynamically adjusts the number of threads based on the available CPU cores. This can be achieved using Node.js's os module to determine the number of CPU cores and then configuring the thread pool accordingly.
Benefits:
- Improved performance by efficiently utilizing CPU resources.
- Reduced overhead from managing too many threads.
- Better scalability and resource management.
Priority: Medium
This issue will help track the progress of this enhancement and ensure it is addressed in a future update.
No description provided.