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

[2.x] Rewrite documentation search page internals to be based on dynamic pages instead of a build task #1498

Merged
merged 70 commits into from
Feb 8, 2024

Conversation

caendesilva
Copy link
Member

@caendesilva caendesilva commented Dec 13, 2023

Abstract

Changes the internals of how the documentation search index and full search page are handled. This is a quite extensive refactor, as part of v2, and is basically how I would/should have implemented this feature from the start, which I couldn't because the original feature predates the improved code API we have now.

Previously, these were generated in a post-build task, with this change, they are now full page objects within the kernel page and route collection.

This targets v2 as while the generated sites won't change, there are several side-effects of this change. These effects are desirable however, and are the reason for the change. But they are sideffects, as can be seen by the amount of files being adapted in this PR.

The biggest change, and the reason behind wanting to make this change, is that with the build tasks, HydePHP has no real knowledge of the files as they are not present in the kernel. This means we've had to make hacky patches for example to serve the pages from the realtime compiler, leading to issues like hydephp/realtime-compiler#19.

By bringing these pages into the kernel, we can serve them directly in the compiler. However, this also means that two new pages are added to the kernel, hence the side effects that could break other integrations. (And it affects tests that don't expect the new pages, hence one of the reasons behind the large amount of files changed in the PR as tests needed to be adapted). Still, the impact is low, as I don't think that many people, if any, have extended the build task. The main impact noticeable to users would be the implicit changes like the pages showing up in the dashboard and route list command.

Release strategy

This contains breaking changes, and thus targets v2.

For the code being deleted, we will want to cherry pick the deprecations to master for a minor release version.

Since this also has a realtime compiler BC break, that will be bumped to a new major version as well.

Breaking changes

High impact:

Low impact:

  • The GenerateSearch post-build task has been removed. If you have previously extended or customized this class,
    you will need to adapt your code, as the search index files are now handled implicitly during the standard build process,
    as the search pages are now added to the kernel page and route collection. ([2.x] Rewrite documentation search page internals to be based on dynamic pages instead of a build task #1498)
  • If your site has a custom documentation search page, for example _docs/search.md or _pages/docs/search.blade.php,
    that page will no longer be build when using the specific build:search command. It will, of course,
    be built using the standard build command. 82dc71f
  • In the highly unlikely event your site customizes any of the search pages by replacing them in the kernel route collection,
    you would now need to do that in the kernel page collection due to the search pages being generated earlier in the lifecycle.
    82dc71f

@caendesilva caendesilva force-pushed the v2/refactor-search-index-handling branch from 31c5c7f to abac409 Compare December 13, 2023 15:14
@caendesilva caendesilva force-pushed the v2/refactor-search-index-handling branch from abac409 to 697e016 Compare December 13, 2023 15:15
Copy link

codecov bot commented Dec 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d52c996) 100.00% compared to head (99a235f) 99.95%.
Report is 38 commits behind head on 2.x-dev.

Additional details and impacted files
@@              Coverage Diff              @@
##             2.x-dev    #1498      +/-   ##
=============================================
- Coverage     100.00%   99.95%   -0.05%     
- Complexity      1740     1744       +4     
=============================================
  Files            180      180              
  Lines           4715     4723       +8     
=============================================
+ Hits            4715     4721       +6     
- Misses             0        2       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@caendesilva caendesilva force-pushed the v2/refactor-search-index-handling branch 2 times, most recently from b83ba27 to 70236af Compare December 13, 2023 17:02
@caendesilva caendesilva changed the title Refactor search index handling Refactor documentation search to be based on dynamic pages instead of a build task Dec 13, 2023
@caendesilva caendesilva force-pushed the v2/refactor-search-index-handling branch from a2f6b06 to 019dd1d Compare December 13, 2023 19:49
@caendesilva caendesilva force-pushed the v2/refactor-search-index-handling branch from 019dd1d to d8c30ba Compare December 13, 2023 20:07
Could cause issues if the user wants to document their search page, maybe we tie it into the create_search_page config option? Because if they have a custom search page, they would want to disable that feature, and then this should also be disabled.
Was probably added as a blanket to include build task output. Can't find any info in the Git log. Since it's unclear, and removing it makes the test pass, that's exactly what I'm going to do.
@caendesilva caendesilva force-pushed the v2/refactor-search-index-handling branch from d8c30ba to 49de93d Compare December 13, 2023 20:09
caendesilva added a commit that referenced this pull request Dec 14, 2023
> Because if they have a custom search page, they would want to disable that feature, and then this should also be disabled.

See #1498 (comment)
> Because if they have a custom search page, they would want to disable that feature, and then this should also be disabled.

See #1498 (comment)
@caendesilva caendesilva force-pushed the v2/refactor-search-index-handling branch from 95dfaf1 to 3cb7f40 Compare December 14, 2023 09:58
@caendesilva caendesilva force-pushed the v2/refactor-search-index-handling branch from 0e757f6 to 2d46f6b Compare December 18, 2023 11:05
Same semantics, just cleaner
@caendesilva caendesilva changed the title Refactor documentation search to be based on dynamic pages instead of a build task Rewire documentation search page internals to be based on dynamic pages instead of a build task Dec 18, 2023
@caendesilva caendesilva changed the title Rewire documentation search page internals to be based on dynamic pages instead of a build task Rewrite documentation search page internals to be based on dynamic pages instead of a build task Dec 18, 2023
Now it uses the same code as the default parent outputPath helper but with a different extension
Since it is ignored here it's weird to require it
@caendesilva caendesilva force-pushed the v2/refactor-search-index-handling branch from 692261a to d81a2ab Compare December 18, 2023 18:51
caendesilva added a commit that referenced this pull request Dec 18, 2023
Since saving to disk is handled by the build process, this class now just needs to be responsible for generating the JSON. See #1498
Since saving to disk is handled by the build process, this class now just needs to be responsible for generating the JSON. See #1498
@caendesilva caendesilva force-pushed the v2/refactor-search-index-handling branch from ba44421 to 1c37851 Compare December 18, 2023 21:46
@caendesilva caendesilva force-pushed the v2/refactor-search-index-handling branch from ca29506 to 99a235f Compare February 7, 2024 20:03
@caendesilva caendesilva marked this pull request as ready for review February 7, 2024 20:12
@caendesilva caendesilva added this to the v2 milestone Feb 8, 2024
@caendesilva caendesilva merged commit bcac53a into 2.x-dev Feb 8, 2024
7 checks passed
@caendesilva caendesilva deleted the v2/refactor-search-index-handling branch February 8, 2024 10:32
@caendesilva caendesilva changed the title Rewrite documentation search page internals to be based on dynamic pages instead of a build task [v2.0] Rewrite documentation search page internals to be based on dynamic pages instead of a build task Feb 8, 2024
@caendesilva caendesilva changed the title [v2.0] Rewrite documentation search page internals to be based on dynamic pages instead of a build task [2.x] Rewrite documentation search page internals to be based on dynamic pages instead of a build task Feb 13, 2024
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.

1 participant