-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Conversation
* Test pages setup * More test page scaffolding * Tests for development-runtime * DRY raw strings with shared data pattern * Production runtime tests * Remove override test, we don't need it
9bdbc22
to
e3f2571
Compare
Can you revert the image changes for contentful, e.g. |
e2e-tests/development-runtime/cypress/integration/head-function-export/correct-props.js
Outdated
Show resolved
Hide resolved
e2e-tests/development-runtime/cypress/integration/head-function-export/html-insertion.js
Outdated
Show resolved
Hide resolved
packages/gatsby/cache-dir/fast-refresh-overlay/components/runtime-errors.js
Show resolved
Hide resolved
packages/gatsby/src/utils/eslint-rules/__tests__/limited-exports-page-templates.ts
Show resolved
Hide resolved
…n-export/correct-props.js Co-authored-by: Lennart <lekoarts@gmail.com>
…ime-errors.js Co-authored-by: Lennart <lekoarts@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cy.getTestElement(`style`).should(`contain`, data.static.style) | ||
cy.getTestElement(`link`) | ||
.invoke(`attr`, `href`) | ||
.should(`equal`, data.static.link) |
There was a problem hiding this comment.
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. 😄
There was a problem hiding this comment.
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.
Description
Link to RFC
Todo
tsx
pages - fix(head-function-export): Make tsx pages work #36114.tsx
pages)Documentation
#36125