Adds support for synchronously updated tags (Closes #291) #297
Conversation
|
|
Codecov Report
@@ Coverage Diff @@
## master #297 +/- ##
==========================================
+ Coverage 98.94% 98.96% +0.02%
==========================================
Files 3 3
Lines 285 291 +6
==========================================
+ Hits 282 288 +6
Misses 3 3
Continue to review full report at Codecov.
|
jaysoo
force-pushed
the
issues/291-fouc
branch
2 times, most recently
from
d0943b0 to
0f1cbdb
Compare
test/HelmetDeclarativeTest.js
Outdated
| it("executes synchronously when defer={true} and async otherwise", done => { | ||
| ReactDOM.render( | ||
| <Helmet> | ||
| <script defer={false}> |
There was a problem hiding this comment.
defer is (I believe) specific to the <script> tag. So this solution wouldn't work for style or any other tag. Perhaps it makes more sense to bubble up the attribute to the <Helmet> wrapper? That would give devs the flexibility to sync-render the entire Helmet instance, rather than the individual tag.
<Helmet defer={false}>
There was a problem hiding this comment.
Option 2 would be to keep this syntax, but strip out invalid defer attributes before we write to the DOM.
There was a problem hiding this comment.
Good point. Maybe it should just go to <Helmet>, then the user can decide whether to separate tags into the defer <Helmet> or not. Would make the API easier to support as well.
I'll redo the PR today.
… sync or deferred. Default is defer={true}. Closes nfl#291
|
@doctyper Update the PR so that <div>
<Helmet defer={false}>
{/* sync tags... */}
</Helmet>
<Helmet>
{/* async tags... */}
</Helmet>
</div> |
|
Hey! Do you have any plans to merge it? |
|
@cwelch5 could you please publish new npm version with merged changes? 🙏 |
|
Great! but when i use link tag. it dose work. css will be load at first. but not work so well. |
| delete window.__spy__; | ||
| }); | ||
|
|
||
| it("executes synchronously when defer={true} and async otherwise", done => { |
There was a problem hiding this comment.
shouldn't this be executes synchronously when defer={false} instead?
This PR is to address the issue with FOUC as outlined in #291.
The code changes involve splitting the updates into two groups: deferred (default), and sync. The sync behaviour is opt-in using the new
defer={false}prop onHelmet.Two new tests have been added to both declarative and original API. They both test that updates can happen before
requestIdleCallbackand during.