-
Notifications
You must be signed in to change notification settings - Fork 457
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
Feature: Add Support for Running Flet with Custom Components and Client Compilation #3883
base: main
Are you sure you want to change the base?
Conversation
Reviewer's Guide by SourceryThis pull request introduces support for running Flet applications with custom components and client compilation. It adds a new ClientCompiler class within the run command, which allows for seamless debugging of Flet applications with custom components. The implementation includes automatic detection and handling of custom dependencies, client recompilation, and environment variable setup. File-Level Changes
Tips
|
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.
Hey @andersou - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider refactoring shared functionality between the new ClientCompiler and the existing build command into a common module to reduce code duplication and improve maintainability.
- The PR introduces a significant amount of new code. In the future, consider breaking large features into smaller, more focused pull requests for easier review and integration.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
|
||
return r | ||
|
||
def compile_client(self) -> None: |
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.
suggestion: Consider breaking down the compile_client method into smaller, focused methods
The compile_client method is quite long and handles multiple responsibilities. Breaking it down into smaller methods could improve readability and maintainability.
def compile_client(self) -> None:
self._setup_compilation_environment()
self._compile_flutter_app()
self._handle_compilation_result()
def _setup_compilation_environment(self) -> None:
self.flutter_dir = None
no_rich_output = get_bool_env_var("FLET_CLI_NO_RICH_OUTPUT")
def _compile_flutter_app(self) -> None:
# Implementation details
def _handle_compilation_result(self) -> None:
# Implementation details
f"Find it in [cyan]{rel_out_dir}[/cyan] directory. {self.emojis['directory']}", | ||
) | ||
|
||
def find_flutter_batch(self, exe_filename: str): |
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.
suggestion: Improve robustness of find_flutter_batch method across different systems
The current implementation makes assumptions about file extensions that might not hold true across all systems. Consider implementing a more robust check that works consistently across different operating systems and Flutter installations.
def find_flutter_batch(self):
for exe_name in ['flutter', 'flutter.bat', 'flutter.exe']:
batch_path = shutil.which(exe_name)
if batch_path:
return batch_path
return None
console = Console(log_path=False, style=Style(color="green", bold=True)) | ||
|
||
|
||
class ClientCompiler: |
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.
issue (complexity): Consider refactoring the ClientCompiler class to improve modularity and maintainability.
The ClientCompiler class, while necessary, could benefit from some refactoring to reduce complexity and improve maintainability. Here are some suggestions:
- Break down the compile_client method into smaller, focused methods:
class ClientCompiler:
def compile_client(self) -> None:
self.setup_environment()
self.create_flutter_project()
self.build_flutter_app()
self.copy_build_results()
self.cleanup()
def setup_environment(self):
# Initialize variables, find Flutter and Dart executables
...
def create_flutter_project(self):
# Create Flutter project from template
...
def build_flutter_app(self):
# Run flutter build command
...
def copy_build_results(self):
# Copy build results to output directory
...
- Use more descriptive variable names and add comments for complex operations:
class ClientCompiler:
def setup_environment(self):
# Find Flutter and Dart executables
self.flutter_executable = self.find_flutter_executable("flutter")
self.dart_executable = self.find_flutter_executable("dart")
# Create temporary directory for Flutter project
self.flutter_project_dir = Path(tempfile.gettempdir()).joinpath(
f"flet_flutter_build_{random_string(10)}"
)
self.flutter_project_dir.mkdir(exist_ok=True)
def find_flutter_executable(self, exe_name: str) -> str:
"""Find Flutter or Dart executable in PATH."""
executable_path = shutil.which(exe_name)
if not executable_path:
self.cleanup(
1,
f"`{exe_name}` command is not available in PATH. Install Flutter SDK.",
)
return executable_path
- Consider using a configuration object to store build settings:
from dataclasses import dataclass
@dataclass
class BuildConfig:
target_platform: str
web_renderer: str
verbose: int
flutter_dependencies: dict
class ClientCompiler:
def __init__(self, config: BuildConfig):
self.config = config
...
def build_flutter_app(self):
build_args = [
self.flutter_executable,
"build",
self.platforms[self.config.target_platform]["build_command"],
]
if self.config.verbose > 1:
build_args.append("--verbose")
...
These changes would make the code more modular, easier to read, and simpler to maintain without sacrificing functionality.
Wow, serious stuff! I've been hoping to implement a new Also, in the next release we are going to support I will review your PR when continue working on Flet 0.25. |
Thank you, Feodor, I’m trying to collaborate and studying the structure of Flet extensively. I’m working to help you all advance the project. If you’d like, feel free to include me in the discussions. I’d love to participate and contribute further to the project. |
Description
Key Changes:
Client Template Creation: A new client template has been created based on the cookiecutter template. This serves as the foundation for generating a custom client environment.
Client Compiler Addition: Introduced a
ClientCompiler
within therun
command. This compiler is a streamlined version of the existingbuild
command, optimized for efficiency.Pubspec.yaml Handling: When executing Flet with the
flet run
command, if apubspec.yaml
file is detected in the same directory as the script (indicating custom dependencies), the script will automatically recompile. The generated client will be placed in a.flet
directory, and environment variables (FLET_WEB_PATH
andFLET_VIEW_PATH
) will be set to point to the newly generated client.This enhancement allows the
flet run
mode to debug with custom components seamlessly.Benefits
How to Test
pubspec.yaml
file with custom dependencies in the same directory as your Flet script.flet run
command.Fixes #3878
Test Code
# Test code for the review of this PR
Type of change
Checklist:
I signed the CLA.
My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
My changes generate no new warnings
New and existing tests pass locally with my changes
Additional details
Summary by Sourcery
Add support for running Flet with custom components by introducing a new client template and a ClientCompiler class. Enable automatic recompilation when custom dependencies are detected, and set environment variables to use the custom client.
New Features:
Enhancements: