Skip to content

Conversation

@lebalz
Copy link
Contributor

@lebalz lebalz commented Nov 26, 2025

Pre-flight checklist

  • I have read the Contributing Guidelines on pull requests.
  • If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
  • If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

see #11570

Test Plan

How to test this? I checked it locally by editing docusaurus.config.ts with all possible combinations...

Tested Configurations

Valid Configs: ✅

{
  headTags: [
    {
        tagName: 'meta',
        attributes: {
          id: 'foo',
        }
    }
  ]
}
{
  headTags: [
    {
        tagName: 'custom-element',
        customElement: true
    }
  ]
}
{
  headTags: [
    {
        tagName: 'custom-element',
        attributes: {
          id: 'foo',
        },
        customElement: true
    }
  ]
}

Invalid Configs ❌

{
  headTags: [
    {
        tagName: 'meta'
    }
  ]
}
{
  headTags: [
    {
        tagName: 'meta',
        customElement: false
    }
  ]
}

Test links

Deploy preview: https://deploy-preview-_____--docusaurus-2.netlify.app/

Related issues/PRs

#11570

@meta-cla meta-cla bot added the CLA Signed Signed Facebook CLA label Nov 26, 2025
@netlify
Copy link

netlify bot commented Nov 26, 2025

[V2]

Name Link
🔨 Latest commit 4c60a50
🔍 Latest deploy log https://app.netlify.com/projects/docusaurus-2/deploys/6928248a15752b000890f91c
😎 Deploy Preview https://deploy-preview-11571--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

I'm adding a little unit test

Also unfortunate that the tag validation logic is spread in 2 different places, but that can be refactored in a separate refactor PR.

An array of tags that will be inserted in the HTML `<head>`. The values must be objects that contain two properties; `tagName` and `attributes`. `tagName` must be a string that determines the tag being created; eg `"link"`. `attributes` must be an attribute-value map. When custom html elements are needed, set `customElement: true`.
- Type: `{ tagName: string; attributes: Object; }[]`
- Type: `({ tagName: string; attributes: Object; } | { tagName: string; attributes?: Object; customElement: true; })[]`
Copy link
Collaborator

@slorber slorber Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to avoid the union type in docs. Although more accurate, it could make the docs harder to read for the usual case where devs are not using custom elements

Although we can be flexible, I'm not sure what could be the value of adding a custom element in head if it doesn't have any attribute. It would only be a "marker" somehow, but it's not really your use-case. I think it's fine if we keep documenting it as if the attributes were required (the object can be empty anyway)

@slorber slorber added the pr: new feature This PR adds a new API or behavior. label Nov 27, 2025
@slorber
Copy link
Collaborator

slorber commented Nov 27, 2025

Not sure why this PR doesn't trigger our CI jobs

Maybe it's because you created the branch from main (it's better to create a dedicated branch in the future).

Anyway, I fixed the test failures and will merge

@slorber slorber merged commit c6a86ff into facebook:main Nov 27, 2025
4 checks passed
@lebalz
Copy link
Contributor Author

lebalz commented Nov 27, 2025

Thanks for the additions, suggestions, and advice — I’ll make sure to apply them next time :)

@slorber
Copy link
Collaborator

slorber commented Nov 27, 2025

np 👍

The GH actions problem is something else, looks to apply to many Meta repo but nobody told me anything 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Signed Facebook CLA pr: new feature This PR adds a new API or behavior.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants