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

onload is not working because is getting rendered as string #202

Open
eduardocque opened this issue Nov 24, 2023 · 0 comments
Open

onload is not working because is getting rendered as string #202

eduardocque opened this issue Nov 24, 2023 · 0 comments

Comments

@eduardocque
Copy link

eduardocque commented Nov 24, 2023

Hi @staylor , i was testing the library and the reason why onload is not working is because is getting rendered as string instead the reference,

https://developer.mozilla.org/en-US/docs/Web/API/Element/setAttribute#value (check this one for references)

checking the next code
src/client.ts method updateTags

for (const attribute in tag) {
        if (Object.prototype.hasOwnProperty.call(tag, attribute)) {
          if (attribute === TAG_PROPERTIES.INNER_HTML) {
            newElement.innerHTML = tag.innerHTML;
          } else if (attribute === TAG_PROPERTIES.CSS_TEXT) {
            // This seems like a CSSImportRuleDeclaration?
            // @ts-ignore
            if (newElement.styleSheet) {
              // @ts-ignore
              newElement.styleSheet.cssText = tag.cssText;
            } else {
              // @ts-ignore
              newElement.appendChild(document.createTextNode(tag.cssText));
            }
          } else {
            const attr = attribute as keyof HTMLElement;
            const value = typeof tag[attr] === 'undefined' ? '' : tag[attr];
            newElement.setAttribute(attribute, value as string);
          }
        }
      }

if we notice, onload will be added with newElement.setAttribute that one is good for strings, but if we need to pass down onload or any other event should be a direct assignation for example

newElement['onload'] = callback; // this way works and is not going to be rendered as string

in this way works perfectly, i havent make a PR due that probably you want to improve this code example

with that change we can do things like this

const fancyLoad = useCallback(e => console.log('Hello World'), []);

// JSX 
<Helmet>
  <script src="niceUrlHere" type="text/javascript" onload={fancyLoad} />
</Helmet>

probably to keep linters happy i will pass the prop as onLoad and internally rename it to onload for javascript standards

in this way we can solve it

a proposal way for the fix

for (const attribute in tag) {
        if (Object.prototype.hasOwnProperty.call(tag, attribute)) {
          if (attribute === TAG_PROPERTIES.INNER_HTML) {
            newElement.innerHTML = tag.innerHTML;
          } else if (attribute === TAG_PROPERTIES.CSS_TEXT) {
            // This seems like a CSSImportRuleDeclaration?
            // @ts-ignore
            if (newElement.styleSheet) {
              // @ts-ignore
              newElement.styleSheet.cssText = tag.cssText;
            } else {
              // @ts-ignore
              newElement.appendChild(document.createTextNode(tag.cssText));
            }
          } else if (typeof tag[attr] === 'function') {
            // Events Here (example fix) will require more tests
            const attr = attribute as keyof HTMLElement;
            const value = typeof tag[attr] === 'undefined' ? '' : tag[attr];
            newElement[attribute] = value;
          } else {
            const attr = attribute as keyof HTMLElement;
            const value = typeof tag[attr] === 'undefined' ? '' : tag[attr];
            newElement.setAttribute(attribute, value as string);
          }
        }
      }
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

No branches or pull requests

1 participant