-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
caendesilva
force-pushed
the
v2/refactor-search-index-handling
branch
from
December 13, 2023 15:14
31c5c7f
to
abac409
Compare
caendesilva
force-pushed
the
v2/refactor-search-index-handling
branch
from
December 13, 2023 15:15
abac409
to
697e016
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
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. |
caendesilva
force-pushed
the
v2/refactor-search-index-handling
branch
2 times, most recently
from
December 13, 2023 17:02
b83ba27
to
70236af
Compare
caendesilva
commented
Dec 13, 2023
packages/framework/src/Framework/Actions/GeneratesDocumentationSearchIndex.php
Outdated
Show resolved
Hide resolved
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
force-pushed
the
v2/refactor-search-index-handling
branch
from
December 13, 2023 19:49
a2f6b06
to
019dd1d
Compare
caendesilva
force-pushed
the
v2/refactor-search-index-handling
branch
from
December 13, 2023 20:07
019dd1d
to
d8c30ba
Compare
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
force-pushed
the
v2/refactor-search-index-handling
branch
from
December 13, 2023 20:09
d8c30ba
to
49de93d
Compare
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
force-pushed
the
v2/refactor-search-index-handling
branch
from
December 14, 2023 09:58
95dfaf1
to
3cb7f40
Compare
caendesilva
force-pushed
the
v2/refactor-search-index-handling
branch
from
December 18, 2023 11:05
0e757f6
to
2d46f6b
Compare
Same semantics, just cleaner
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
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
force-pushed
the
v2/refactor-search-index-handling
branch
from
December 18, 2023 18:51
692261a
to
d81a2ab
Compare
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
commented
Dec 18, 2023
packages/framework/resources/views/components/docs/documentation-article.blade.php
Outdated
Show resolved
Hide resolved
caendesilva
force-pushed
the
v2/refactor-search-index-handling
branch
from
December 18, 2023 21:46
ba44421
to
1c37851
Compare
The change in behaviour has been documented, so we can remove the disabled legacy assertions
caendesilva
force-pushed
the
v2/refactor-search-index-handling
branch
from
February 7, 2024 20:03
ca29506
to
99a235f
Compare
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
This was referenced Feb 8, 2024
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
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)
_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. 82dc71fyou would now need to do that in the kernel page collection due to the search pages being generated earlier in the lifecycle.
82dc71f