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

feat(gatsby): Gatsby Head API #35980

Merged
merged 69 commits into from Jul 14, 2022
Merged

feat(gatsby): Gatsby Head API #35980

merged 69 commits into from Jul 14, 2022

Conversation

marvinjude
Copy link
Contributor

@marvinjude marvinjude commented Jun 23, 2022

Description

Link to RFC

Todo

Documentation

#36125

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jun 23, 2022
@marvinjude marvinjude added this to the Metadata Management milestone Jun 23, 2022
@marvinjude marvinjude added type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement. and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Jun 24, 2022
@LekoArts LekoArts added the topic: DX Developer Experience (e.g. Fast Refresh, i18n, SSR, page creation, starters) label Jun 24, 2022
@LekoArts
Copy link
Contributor

Can you revert the image changes for contentful, e.g. packages/gatsby-source-contentful/src/__tests__/23df7612602d69edfc3eaf89c8c02248/347966-contentful-logo-wordmark-dark (1)-4cd185-original-1582664935 (1).png

@marvinjude marvinjude marked this pull request as ready for review July 13, 2022 09:31
Copy link
Contributor

@imjoshin imjoshin left a comment

Choose a reason for hiding this comment

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

Excited

cy.getTestElement(`style`).should(`contain`, data.static.style)
cy.getTestElement(`link`)
.invoke(`attr`, `href`)
.should(`equal`, data.static.link)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit-picking: We have these assertions a lot, and adding/removing functionality in the future will be quite a task to update these. We could pull these assertions into a helper for this e2e suite only... Something like:

export const assertHeadElementAttributes({base, title, ...}) {
    cy.getTestElement(`base`)
    ...
}

Feel free to ignore. 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That'll be quite handy! 'style', 'link' and co above are test ids though not tag names

We could have something that asserts for text and attributes for a test id like so:

export const assertElementAttributesAndText = elements => {
  for (const { id, text, attributes } of elements) {
    if (text) cy.getTestElement(id).should("have.text", text)
    for (const [key, value] of Object.entries(attributes)) {
      cy.getTestElement(id).invoke("attr", key).should("equal", value)
    }
  }
}

const elements = [
  {
    id: "title",
    text: "Head function export should work",
    attributes: {
      content: "title",
    },
  },
]

assertElementAttributesAndText(elements)

Edit : Seem like an issue with all our tests though where we repeat things. Also when you want something like shoud.not then our abstraction may look messy.

We can add something that works for common cases in out test some other time via a non-metadata related PR.

packages/gatsby/cache-dir/react-dom-utils.js Show resolved Hide resolved
packages/gatsby/package.json Show resolved Hide resolved
@LekoArts LekoArts changed the title feat(gatsby): Metadata management feat(gatsby): Gatsby Head API Jul 14, 2022
@LekoArts LekoArts merged commit 40810c4 into master Jul 14, 2022
@LekoArts LekoArts deleted the feat/metadata-management branch July 14, 2022 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: DX Developer Experience (e.g. Fast Refresh, i18n, SSR, page creation, starters) type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants