Fix/support empty initial style block

[anied]

Fix: Support empty initial style block

Overview

When attempting to provide an initial <style/> element for goober to leverage (instead of generating its own), it currently fails if that style block is empty (<style id="_goober"></style>). Depending on the setup, simply adding a comment to the style block may not be sufficient, as the build might strip out comments and white space. (Specifically, .firstChild property of an empty <style/> element is null, so when extractCss attempts to access the .data property of null it throws an error.)

This change updates getSheet to check if the <style/> element is empty, and inserts a comment into it before returning the .firstChild property of the element. When using this approach the code functions correctly in the above scenario.

There were no new failing tests with this change. Please update in comments any additional steps needed to get this PR ready for merge.

Change Log

• Updates getSheet to check if the <style/> element is empty, and inserts a comment into it before returning the .firstChild property of the element, allowing an initial <style/> block provided to goober to be empty
• package-lock.json changes from running npm install

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
goober-rocks	✅ Ready (Inspect)	Visit Preview	Nov 8, 2022 at 3:57PM (UTC)

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit a1d7d1b:

Sandbox	Source
Vanilla	Configuration

[cristianbote]

Thank you @anied for this! I've seen folks get annoyed with this so this is truly needed.

Should we do the first element check only for the existing tags? Meaning the assign call adds the missing tag with the empty space. Which brings me to the next question, instead of a comment could we use an empty space?

[anied]

Hello @cristianbote !

Should we do the first element check only for the existing tags? Meaning the assign call adds the missing tag with the empty space.

I'm not sure I completely follow this question. It is true that the if (!styleElem.firstChild) styleElem.innerHTML = '/*goober*/'; line is superfluous for cases in which this is how the style block is added:

Object.assign((target || document.head).appendChild(document.createElement('style')), {
  innerHTML: ' ',
  id: GOOBER_ID
});

However, I knew that there was a big focus on keeping the size of the library down, so I figured a potential superfluous check would be preferable to a larger bundle; I'm happy to write this to avoid the superfluous call if that's desired. Am I understanding the ask correctly?

Which brings me to the next question, instead of a comment could we use an empty space?

Yes, I believe this should be totally fine, but I could verify it later.

[cristianbote]

Ah sorry for not giving a more deeper explanation. I'll do my best to add more details.

const currentStyleTag = target ? target.querySelector('#' + GOOBER_ID) : window[GOOBER_ID];

if (currentStyleTag) {
  if (!currentStyleTag.firstChild) {
    currentStyleTag.innerHTML = ' ';
  }

  return currentStyleTag.firstChild;
}

return Object.assign((target || document.head).appendChild(document.createElement('style')), {
  innerHTML: ' ',
  id: GOOBER_ID
}).firstChild;

Not sure how much size this would add though 😅

[anied]

Hahaha, OK, yes-- this was my first instinct, but seeing the push for keeping the size down was the reason I went with the approach I did. I'm happy to rework it to be a bit more readable and efficient at the expense of a few more bytes if you thinks that's acceptable.

[cristianbote]

I'm not at my desk now but frankly I think what we have here is what's gonna get generated at the end as well.

[anied]

OK, cool-- I can refactor to this approach in a few hours. Thanks!