Skip to content

Send error information to event #108

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

Merged
merged 5 commits into from
May 19, 2025
Merged

Send error information to event #108

merged 5 commits into from
May 19, 2025

Conversation

manuelpuyol
Copy link
Contributor

@manuelpuyol manuelpuyol commented May 19, 2025

Sending the actual error will help users understand why the fragment is failing

@manuelpuyol manuelpuyol changed the title Send error to event Send error information to event May 19, 2025
@manuelpuyol manuelpuyol marked this pull request as ready for review May 19, 2025 21:29
@Copilot Copilot AI review requested due to automatic review settings May 19, 2025 21:29
@manuelpuyol manuelpuyol requested a review from a team as a code owner May 19, 2025 21:29
@@ -9,11 +9,11 @@
{
"kind": "variable",
"name": "IncludeFragmentElement",
"default": "class extends HTMLElement {\n constructor() {\n super(...arguments);\n _IncludeFragmentElement_instances.add(this);\n _IncludeFragmentElement_busy.set(this, false);\n _IncludeFragmentElement_observer.set(this, new IntersectionObserver((entries) => {\n for (const entry of entries) {\n if (entry.isIntersecting) {\n const { target } = entry;\n __classPrivateFieldGet(this, _IncludeFragmentElement_observer, \"f\").unobserve(target);\n if (!(target instanceof IncludeFragmentElement))\n return;\n if (target.loading === \"lazy\") {\n __classPrivateFieldGet(this, _IncludeFragmentElement_instances, \"m\", _IncludeFragmentElement_handleData).call(this);\n }\n }\n }\n }, {\n rootMargin: \"0px 0px 256px 0px\",\n threshold: 0.01\n }));\n }\n static define(tag = \"include-fragment\", registry = customElements) {\n registry.define(tag, this);\n return this;\n }\n static setCSPTrustedTypesPolicy(policy) {\n cspTrustedTypesPolicyPromise = policy === null ? policy : Promise.resolve(policy);\n }\n static get observedAttributes() {\n return [\"src\", \"loading\"];\n }\n get src() {\n const src = this.getAttribute(\"src\");\n if (src) {\n const link = this.ownerDocument.createElement(\"a\");\n link.href = src;\n return link.href;\n } else {\n return \"\";\n }\n }\n set src(val) {\n this.setAttribute(\"src\", val);\n }\n get loading() {\n if (this.getAttribute(\"loading\") === \"lazy\")\n return \"lazy\";\n return \"eager\";\n }\n set loading(value) {\n this.setAttribute(\"loading\", value);\n }\n get accept() {\n return this.getAttribute(\"accept\") || \"\";\n }\n set accept(val) {\n this.setAttribute(\"accept\", val);\n }\n get data() {\n return __classPrivateFieldGet(this, _IncludeFragmentElement_instances, \"m\", _IncludeFragmentElement_getStringOrErrorData).call(this);\n }\n attributeChangedCallback(attribute, oldVal) {\n if (attribute === \"src\") {\n if (this.isConnected && this.loading === \"eager\") {\n __classPrivateFieldGet(this, _IncludeFragmentElement_instances, \"m\", _IncludeFragmentElement_handleData).call(this);\n }\n } else if (attribute === \"loading\") {\n if (this.isConnected && oldVal !== \"eager\" && this.loading === \"eager\") {\n __classPrivateFieldGet(this, _IncludeFragmentElement_instances, \"m\", _IncludeFragmentElement_handleData).call(this);\n }\n }\n }\n connectedCallback() {\n if (!this.shadowRoot) {\n this.attachShadow({ mode: \"open\" });\n const style = document.createElement(\"style\");\n style.textContent = `:host {display: block;}`;\n this.shadowRoot.append(style, document.createElement(\"slot\"));\n }\n if (this.src && this.loading === \"eager\") {\n __classPrivateFieldGet(this, _IncludeFragmentElement_instances, \"m\", _IncludeFragmentElement_handleData).call(this);\n }\n if (this.loading === \"lazy\") {\n __classPrivateFieldGet(this, _IncludeFragmentElement_observer, \"f\").observe(this);\n }\n }\n request() {\n const src = this.src;\n if (!src) {\n throw new Error(\"missing src\");\n }\n return new Request(src, {\n method: \"GET\",\n credentials: \"same-origin\",\n headers: {\n Accept: this.accept || \"text/html\"\n }\n });\n }\n load() {\n return __classPrivateFieldGet(this, _IncludeFragmentElement_instances, \"m\", _IncludeFragmentElement_getStringOrErrorData).call(this);\n }\n fetch(request) {\n return fetch(request);\n }\n refetch() {\n privateData.delete(this);\n __classPrivateFieldGet(this, _IncludeFragmentElement_instances, \"m\", _IncludeFragmentElement_handleData).call(this);\n }\n}"
"default": "class _IncludeFragmentElement extends HTMLElement {\n static"
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if this is unusual how it's cut off at static"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ugh idk why this file changed at all

Copy link
Member

Choose a reason for hiding this comment

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

I'm not really sure if it's important? like why would we store code in a json object 🤔

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the IncludeFragmentElement to include the actual error object when dispatching error-related events, improving debuggability for downstream consumers.

  • Updated the private #task method to accept an optional Error and dispatch CustomEvent with detail.error.
  • Modified the error-handling path to pass the caught Error into #task.
  • Added a test assertion for the new event.detail.error and updated component metadata in custom-elements.json.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
test/test.js Added assertion for event.detail.error containing the error text
src/include-fragment-element.ts Extended #task to accept and dispatch error details in events
custom-elements.json Updated metadata to include the new error parameter and renamed defaults
Comments suppressed due to low confidence (1)

custom-elements.json:469

  • There are duplicate test/test.js entries in custom-elements.json; consider removing the redundant module declaration to keep metadata concise.
    { "kind": "javascript-module", "path": "test/test.js", "declarations": [], "exports": [] }

manuelpuyol and others added 2 commits May 19, 2025 14:33
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@manuelpuyol manuelpuyol merged commit 75ac8d9 into main May 19, 2025
4 checks passed
@manuelpuyol manuelpuyol deleted the manuelpuyol-patch-1 branch May 19, 2025 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants