[Preview - Part 2] Basic Framework#1934
Conversation
…thout having to rely on global-scoped variables.
markrandall
force-pushed
the
preview-framework
branch
4 times, most recently
from
dbb6e99 to
6521819
Compare
markrandall
force-pushed
the
preview-framework
branch
from
6521819 to
c0c3b85
Compare
…target of exception
markrandall
requested review from
derickr,
saundefined,
sgolemon and
sy-records
as code owners
| */ | ||
| public static function readContents(string $path): string | ||
| { | ||
| $contents = file_get_contents($path); |
There was a problem hiding this comment.
This may throw an E_WARNING. We can use thecodingmachine/safe to avoid that behaviour. Only checking for false is not sufficient.
There was a problem hiding this comment.
It looks like checking against false is what safe does https://github.com/thecodingmachine/safe/blob/4fbc0088994d486b0012d67116b90825f17a9309/generated/8.1/filesystem.php#L165
There was a problem hiding this comment.
That said, we could potentially bring in League's Flysystem for this, that would give us a consistent API across any future filesystems we need to use.
| /** | ||
| * @return array<string, ReflectionClass<object>> | ||
| */ | ||
| public static function getReflectionClasses(): array |
There was a problem hiding this comment.
I would prefer that we define $cache as property. I am only really in favor of static $var within functions. It's used at some more places.
| } | ||
|
|
||
| /* protect us against doing silly things */ | ||
| BuildTools::$building = true; |
There was a problem hiding this comment.
Probably better to use static in case we ever change the class name.
| public static function getLastModifiedTime(): int | ||
| { | ||
| /* in development mode we always assume that we've recently refreshed our data */ | ||
| if (isset($_ENV['DEVELOPMENT'])) { |
There was a problem hiding this comment.
This will fail if DEVELOPMENT=0. It's better to define a global constant const DEVELOPMENT = true during initialisation.
As for the name. I prefer IS_DEBUG. But that's a minor nit.
| { | ||
| public static function build(BuildContext $context): void | ||
| { | ||
| ini_set('memory_limit', '1G'); |
There was a problem hiding this comment.
Is this really necessary? It's a big chunk of memory.
There was a problem hiding this comment.
What's the downside? Although that should be in the build initializer.
| */ | ||
| function safe(string $html): string | ||
| { | ||
| return htmlspecialchars($html, encoding: 'UTF-8'); |
There was a problem hiding this comment.
The function name safe feels very vague. Can we make this more explicit, like safe_htmlspecialchars.
Though this doesn't seem to be used now while I do see htmlspecialchars being used all over.
2 participants
Builds upon Part 1 - #1931
These core framework components all work together. While they could technically be split up into individual PR I have chosen not to do so due to their tight dependence on each other to deliver real-world functionality.
As we're only writing glue code for industry-standard libraries, rather than building fully custom implementations of routing and services, being able to see them all working together should provide better context for reviewing than if they were all self-contained with no meaningful bigger-picture.
Build Process
Facilitates creating optimized data for the sake of enormously increased performance vs runtime reflection and data processing.
The "build" logic itself is a a simple executor that auto-detects classes that implement the
BuildStepinterface, and invokes their staticbuildmethod, passing in a basic context & logging object. The meat of the build process is contained in those function-specific classes that implementBuildStep.To invoke build, run
php bin/build.phpand the compiled data will be generated invar/, to which reading and writing is encapsulated viaVarCache.Coupling: Used by both router and services to provide essential performance improvements vs runtime scanning and reflection.
External Dependencies: None
Routing
Decouples the physical path of the .php file from its controller code, allows using placeholders / wildcards within routing URLs instead of needing to create large numbers of template-generated files (as is the case with release announcements) e.g.
releases/8_4_1.php.Uses standard attribute-based
#[Route]ing mechanism to detect classes, which are also used by the service layer to register anything with one as a service.Coupling: Depends on build step to compile routing paths without incurring significant performance hit. Used by Kernel for core functionality.
External Dependencies: Symfony Router
Service Layer
Allows registering classes as services, allows inversion of control (IoC) by autowiring service constructor arguments with other specified services.
Coupling: Depends on build step to compile service definitions, without incurring significant performance hit. Depends on routing layer to bootstrap container and create initial controllers. Used by Kernel for core functionality.
External Dependencies: Symfony Dependency Injection / Container Interface
HTTP Kernel
Processes a HTTP request and converts it to a Response, and sends that response to the end-user. Currently because this is a workaround until we can get a fully supported front controller this kernel only gets invoked in the error.php handler, and if it does not match a route it continues on to let the other hundreds of rules in error.php take their turn.
Coupling: Depends on Router, Service Layer
External Dependencies: None