stencil icon indicating copy to clipboard operation
stencil copied to clipboard

fix: prevent broken stylesheet being added to DOM

Open charlieTheBotDev opened this issue 3 years ago • 4 comments

Our component library is being loaded by Jest and we have several pages that don't include any Stencil components but still seem to load the script that adds the hydrated styles. This leads to the 'HYDRATED_CSS' being added to the page on its own, which is invalid as it begins with '{'

Checking for the 'cmpTags' length prevents this from happening

For reference, the error shown is:

  console.error
    Error: Could not parse CSS stylesheet
        at exports.createStylesheet (/node_modules/jsdom/lib/jsdom/living/helpers/stylesheets.js:35:21)
        at HTMLStyleElementImpl._updateAStyleBlock (/node_modules/jsdom/lib/jsdom/living/nodes/HTMLStyleElement-impl.js:68:5)
        at HTMLStyleElementImpl._attach (/node_modules/jsdom/lib/jsdom/living/nodes/HTMLStyleElement-impl.js:19:12)
        at HTMLHeadElementImpl._insert (/node_modules/jsdom/lib/jsdom/living/nodes/Node-impl.js:845:14)
        at HTMLHeadElementImpl._preInsert (/node_modules/jsdom/lib/jsdom/living/nodes/Node-impl.js:768:10)
        at HTMLHeadElementImpl.insertBefore (/node_modules/jsdom/lib/jsdom/living/nodes/Node-impl.js:605:17)
        at HTMLHeadElement.insertBefore (/node_modules/jsdom/lib/jsdom/living/generated/Node.js:370:60)
        at Object.bootstrapLazy (/node_modules/PRIVATE/dist/cjs/index-d7cb340b.js:1265:14)
        at /node_modules/PRIVATE/dist/cjs/loader.cjs.js:17:16
        at processTicksAndRejections (internal/process/task_queues.js:93:5) {visibility:hidden}.hydrated{visibility:inherit}

It attempts to add this to the DOM

<style>
{visibility:hidden}.hydrated{visibility:inherit}
</style>

It's possibly also worth mentioning that this fixes the symptom and there may be an underlying issue causing cmpTags to be empty in the first place (either in my app code or Stencil) but this at least prevents the error

It might be worth investigating and throwing an error / not doing other things if cmpTags is empty?

charlieTheBotDev avatar Feb 10 '21 10:02 charlieTheBotDev

After some debugging, I've found that something is calling defineCustomElements twice, which calls boostrapLazy twice and then subsequently doesn't push anything to cmpTags (because the elements are all defined already which implies they already have the style added to the page)

I'm still not 100% sure what or why defineCustomElements/boostrapLazy is being called twice but it should never be possible to add an invalid stylesheet to the page so I'm reasonably confident this is a good change

charlieTheBotDev avatar Feb 15 '21 09:02 charlieTheBotDev

@c-frater hi, what is the status of the pull request, can you update it so that it can be merged at some point?

@simonhaenisch unfortunately ran into the same problem because a dependency of a dependency also starts the loader and the main application so we also run through the loader process twice and run into the css problem

ReneWerner87 avatar Dec 02 '21 06:12 ReneWerner87

I haven't worked on or with Stencil in a while... can't be of any help here, sorry. Better to ping @splitinfinities who's on the team now.

simonhaenisch avatar Dec 03 '21 08:12 simonhaenisch

@splitinfinities would be cool if we could look at this pull request again

ReneWerner87 avatar Dec 07 '21 07:12 ReneWerner87

Closing this in favor of https://github.com/ionic-team/stencil/pull/3771

rwaskiewicz avatar Nov 10 '23 20:11 rwaskiewicz