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

[SR-15576] [Swift-DocC] Transform-for-static-hosting does not DocC's experimental custom header/footer feature #196

Closed
ethan-kusters opened this issue Dec 9, 2021 · 5 comments · Fixed by #410
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@ethan-kusters
Copy link
Contributor

Previous ID SR-15576
Radar rdar://problem/86298113
Original Reporter @ethan-kusters
Type Bug
Additional Detail from JIRA
Votes 0
Component/s Swift-DocC
Labels Bug
Assignee None
Priority Medium

md5: c735028a1eb87105ac679771fac32b43

cloned to:

  • SR-15750 [Swift-DocC] docc archive transform-for-static-hosting should add support for experimental custom header/footer feature

relates to:

  • SR-15750 [Swift-DocC] docc archive transform-for-static-hosting should add support for experimental custom header/footer feature

Issue Description:

Currently the `transform-for-static-hosting` action does not respect the experimental header/footer that can be provided in a DocC Catalog.

Since `transform-for-static-hosting` is still experimental, this lack of support did not block landing the initial implementation but it should be fixed for user's who want to host their docs in a static environment and take advantage of the experimental custom header/footer support.

The basics of what are needed to add support can be found here: ethan-kusters@119770f.

@ethan-kusters
Copy link
Contributor Author

@swift-ci create

@ethan-kusters
Copy link
Contributor Author

We'll use this bug to track support specifically for `docc convert --transform-for-static-hosting --enable-experimental-custom-templates`.

Support for `docc archive transform-for-static-hosting --enable-experimental-custom-templates` is tracked separately with SR-15750.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 3, 2022
@ethan-kusters
Copy link
Contributor Author

I think this would be a good starter issue. Here's some thoughts on how to get started:

DocC's transform for static hosting logic for docc convert follows this general code path:

  1. ConvertAction.swift#L319: Check if a copy of Swift-DocC Render has been provided and find the URL for the index.html it contains. If the user has provided a custom hosting base path, use the template-index.html and do some string replacement to write out a custom index.html and save that URL. Primary goal of this section is to get the URL for the index.html file that needs to be copied around for static hosting.
  2. ConvertAction.swift#L372: Pass that index.html URL to the convert action's file writer.
  3. ConvertAction.swift#L378: Begin the conversion process.
  4. DocumentationConverter.swift#L293: Documentation converter converts an individual render node and passes it to the file writing consumer.
  5. ConvertFileWritingConsumer.swift#L61: File writing consumer passes the render node to the render node writer.
  6. JSONEncodingRenderNodeWriter.swift#L120: The render node writer creates a copy of the index.html in the currently building DocC archive (provided by step (2)) to a path relative to where the current render node being written in the data directory.

Separately, in ConvertFileWritingConsumer there's logic that picks up any provided custom header/footer templates and injects them in to that same index.html at the root of the DocC archive: ConvertFileWritingConsumer.swift#L191.

Unfortunately, the ConvertFileWritingConsumer is provided the custom header/footer templates after it's already written out all of the render nodes, and thus copied around the index.html without the injected header/footer.

And so we end up in the situation where the index.html file at the root of the emitted archive has the correctly injected custom header/footer, but all of the other index.html files for each individual render node are incorrect.


I believe the simplest fix here would be to just move the call to try outputConsumer.consume(assetsInBundle: bundle) in DocumentationConverter before any calls to try outputConsumer.consume(renderNode: renderNode). This would ensure that whenever the output consumer consumers a render node, the index.html in the output archive is the correct, already transformed one.

An alternative solution would be to move the logic for injecting custom header/footer earlier in the process.

Either way, we'll need to add a test that ensures that when a bundle that includes a custom header/footer is converted with support for static hosting, that custom header/footer is included in all of the copied index.html files. For that, I would create new copy of testConvertWithCustomTemplates() that converts a bundle the includes actual content (that can be done by adding a symbol graph to the folder used as input). Then assert that the each of the copied index.html files also have the expected content just as the current test is asserting that the one at the root does.

@mportiz08
Copy link
Contributor

@ethan-kusters can this be closed with the merge of #410?

@ethan-kusters
Copy link
Contributor Author

@mportiz08 Yes! Thank you for the ping.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants