-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Moves content layer sync to a queue and support selective sync #11767
Conversation
🦋 Changeset detectedLatest commit: 3b9ff82 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@@ -152,6 +152,7 @@ | |||
"esbuild": "^0.21.5", | |||
"estree-walker": "^3.0.3", | |||
"fast-glob": "^3.3.2", | |||
"fastq": "^1.17.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already a transitive dependency via about a million of our deps
c220faf
to
bb617b9
Compare
bb617b9
to
19ef892
Compare
f650f18
to
90a862f
Compare
Missing a changeset. |
if ( | ||
options?.loaders && | ||
(typeof collection.loader !== 'object' || | ||
!options.loaders.includes(collection.loader.name)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I understand !options.loaders.includes(collection.loader.name)
, but I ask just in case: why do we add this check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this will allow selective sync: if loaders
is passed then only loaders named in that list will be refreshed
*/ | ||
|
||
sync(options: RefreshContentOptions = {}): Promise<void> { | ||
return this.#queue.push(options); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pardon the question, I don't know how fastq
works, and the the docs are a bit difficult to grasp. Does push
execute the task by itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, pushing it onto the queue will start the queue running
Changes
Currently if a sync call is made while the content layer is already syncing it is silently ignored. This PR refactors content layer syncing to instead use a queue for these calls. The motivation for this is to enable selective sync, and sync context. These are not publicly exposed yet, but in 5.0 will be available to integrations.
Testing
Tests still pass. Extra tests will be added once the feature is implemented.
Docs
Internal change