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

[json-loader] Don't emit new file node until previous is finished processing #4785

Merged
merged 3 commits into from
Apr 3, 2018

Conversation

KyleAMathews
Copy link
Contributor

This is an experiment to use
xstate to setup state
machines to better handle complex state changes as we sometimes have.

Ideally this happens in core and then gatsby-source-filesystem just has a
simple queue and emits a new file node every time the system returns to
idle.

In a future refactor we'll do that plus refactor other parts of core that
should be handled in a state machine e.g. pages-query-runner.js

This PR also reinforced the need for us to implement
tracing in core / plugins as
that'd make it far far easier to understand what's happening and when.

This is an experiment to use
[xstate](http://davidkpiano.github.io/xstate/docs/#/) to setup state
machines to better handle complex state changes as we sometimes have.

Ideally this happens in core and then gatsby-source-filesystem
just has a simple queue and emits a new file node every time
the system returns to idle.

In a future refactor we'll do that plus refactor other parts of core
that should be handled in a state machine e.g. pages-query-runner.js

This PR also reinforced the need for us to implement
[tracing](https://github.com/jaegertracing/jaeger) in core / plugins
as that'd make it far far easier to understand what's happening and
when.
@ghost ghost assigned KyleAMathews Mar 31, 2018
@ghost ghost added the review label Mar 31, 2018
@KyleAMathews KyleAMathews changed the title Don't emit new file node until previous is finished processing [json-loader] Don't emit new file node until previous is finished processing Mar 31, 2018
@KyleAMathews
Copy link
Contributor Author

Nice article about tracing and why it's so much nicer than logs if you haven't read about it before https://medium.com/opentracing/take-opentracing-for-a-hotrod-ride-f6e3141f7941

@m-allanson
Copy link
Contributor

This looks really nice! Reading this on my phone, are all three CHOKIDAR_ states used?

@KyleAMathews
Copy link
Contributor Author

Nope! Good point — I think I intended to use something with the other states but didn't. I'll just make them NOT_READY and READY.

Nice to see that they were readily understandable!

@pieh
Copy link
Contributor

pieh commented Apr 2, 2018

This looks really nice - very cool that state transitions "definition" is kept in one place to get overview on possible states and general flow. Do You want to merge it as-is or just use it as proof of concept? I guess filesystem is only real source plugin that will update mid develop (refresh hook doesn't seem to be actively used), so this could definitely go in.

Only thing for me apart from extra states pointed out by @m-allanson would be some explanation what problem this solves for people who will try to figure it out later on.

@KyleAMathews
Copy link
Contributor Author

Do You want to merge it as-is or just use it as proof of concept? I guess filesystem is only real source plugin that will update mid develop (refresh hook doesn't seem to be actively used), so this could definitely go in.

Yeah — I was planning on doing a general purpose solution but realized that gatsby-source-filesystem is the only way right now to get Gatsby in this sort of inconsistent state. So build a specific fix now that we can generalize in the future once we understand the problem better and possible solutions better.

I'll add some comments on the states + remove the extra Chokidar states.

@KyleAMathews KyleAMathews merged commit abcb58c into json-loader Apr 3, 2018
@ghost ghost removed the review label Apr 3, 2018
@KyleAMathews KyleAMathews deleted the xstate branch April 3, 2018 17:32
KyleAMathews pushed a commit that referenced this pull request Apr 6, 2018
…ion (aka Ludicrous Mode) (#4555)

* Create placeholder JSON store

* Rename

* Websocket placeholder

* Push query results JSON over websockets

* More descriptive variable name

* Fix queries being overwritten

* Remove eslint-disable flag

* Remove junk

* test require error fix for windows

* dont require json data in sync-require

* dont add layout data to json array multiple times

* initial async loading

* revert saving json directly to public for now

* updated production-app to sync with prop name change in ComponentRenderer

* we load json data via json-loader component in develop and not handling it with webpack import/require

* hashes for json files

* fix preloading, use xhr instead of fetch - for some reason can't force fetch to not create additional request, with any `cache` or `mode` configuration

* dont use full paths in dataPath - remove static/d/ path and .json ext - results in smaller app bundle especially with large ammount of pages

* Enable cached query results to be loaded

* Don't dump all query results out to the client

Instead only push results out if the data is for a path that's currently
being viewed in a client.

* fix preload link to json data

* remove not used function

* remove more not used code

* Update to latest webpack/mini-css-extract-plugin

* don't write new (a)sync-requires.js if components didn't change (#4759)

* create just one websocket client (#4763)

* Filter out duplicate query jobs and create secondary queue for jobs if path already has query in flight

* [json-loader] Don't emit new file node until previous is finished processing (#4785)

* Don't emit new file node until previous is finished processing

This is an experiment to use
[xstate](http://davidkpiano.github.io/xstate/docs/#/) to setup state
machines to better handle complex state changes as we sometimes have.

Ideally this happens in core and then gatsby-source-filesystem
just has a simple queue and emits a new file node every time
the system returns to idle.

In a future refactor we'll do that plus refactor other parts of core
that should be handled in a state machine e.g. pages-query-runner.js

This PR also reinforced the need for us to implement
[tracing](https://github.com/jaegertracing/jaeger) in core / plugins
as that'd make it far far easier to understand what's happening and
when.

* Document state machine and remove extraneous Chokidar states

* Remove console.log

* [json-loader] Only log file events if we're past bootstrap (#4826)

* Don't emit new file node until previous is finished processing

This is an experiment to use
[xstate](http://davidkpiano.github.io/xstate/docs/#/) to setup state
machines to better handle complex state changes as we sometimes have.

Ideally this happens in core and then gatsby-source-filesystem
just has a simple queue and emits a new file node every time
the system returns to idle.

In a future refactor we'll do that plus refactor other parts of core
that should be handled in a state machine e.g. pages-query-runner.js

This PR also reinforced the need for us to implement
[tracing](https://github.com/jaegertracing/jaeger) in core / plugins
as that'd make it far far easier to understand what's happening and
when.

* Document state machine and remove extraneous Chokidar states

* Remove console.log

* Only log file events if we're past bootstrap

* [json-loader] dont recompile on data change - part 2 (#4837)

* prevent adding duplicate redirects

* don't write new `redirects.json` if redirects didn't change

prevents webpack recompilation on data change

* [json-loader] develop - reading results from file improvments (#4850)

* dont emit results for layouts

* [develop] store query results in memory, read json data from file only if we don't have it stored yet (we didn't run this query, but results are cached)

* Add query prioritization based on what page(s) user(s) are on

Query running is sadly not very ludicrous right now on gatsbyjs.org —
not sure why — each markdown file change causes ~20 queries to run but
even with prioritizing the active page's query, it's still ~2 seconds
before the page updates.

This sort of thing will be much easier to debug with tracing support.

* Add initial forward slash

* Actually this is how we add back the initial forward slash
@davidkpiano
Copy link

This is awesome! Just for fun, here's a visualization:

screen shot 2018-04-09 at 9 02 09 am

@KyleAMathews
Copy link
Contributor Author

@davidkpiano hey David! Thanks for stopping by and for the great library! Had a great time watching your talk and reading through your docs and using it to implement this PR! We'll be using xstate a bunch more in Gatsby in the future 💪

Cool visualization! How did you make it?

@davidkpiano
Copy link

It was automatically generated - based on this visualizer, but I'll be releasing an upgraded version at React Finland: https://codepen.io/davidkpiano/pen/ayWKJO

@KyleAMathews
Copy link
Contributor Author

Nice! Looking forward to watching your recent JSConf talk https://www.youtube.com/watch?time_continue=1&v=RqTxtOXcv8Y

mwfrost pushed a commit to mwfrost/gatsby that referenced this pull request Apr 20, 2023
…ion (aka Ludicrous Mode) (gatsbyjs#4555)

* Create placeholder JSON store

* Rename

* Websocket placeholder

* Push query results JSON over websockets

* More descriptive variable name

* Fix queries being overwritten

* Remove eslint-disable flag

* Remove junk

* test require error fix for windows

* dont require json data in sync-require

* dont add layout data to json array multiple times

* initial async loading

* revert saving json directly to public for now

* updated production-app to sync with prop name change in ComponentRenderer

* we load json data via json-loader component in develop and not handling it with webpack import/require

* hashes for json files

* fix preloading, use xhr instead of fetch - for some reason can't force fetch to not create additional request, with any `cache` or `mode` configuration

* dont use full paths in dataPath - remove static/d/ path and .json ext - results in smaller app bundle especially with large ammount of pages

* Enable cached query results to be loaded

* Don't dump all query results out to the client

Instead only push results out if the data is for a path that's currently
being viewed in a client.

* fix preload link to json data

* remove not used function

* remove more not used code

* Update to latest webpack/mini-css-extract-plugin

* don't write new (a)sync-requires.js if components didn't change (gatsbyjs#4759)

* create just one websocket client (gatsbyjs#4763)

* Filter out duplicate query jobs and create secondary queue for jobs if path already has query in flight

* [json-loader] Don't emit new file node until previous is finished processing (gatsbyjs#4785)

* Don't emit new file node until previous is finished processing

This is an experiment to use
[xstate](http://davidkpiano.github.io/xstate/docs/#/) to setup state
machines to better handle complex state changes as we sometimes have.

Ideally this happens in core and then gatsby-source-filesystem
just has a simple queue and emits a new file node every time
the system returns to idle.

In a future refactor we'll do that plus refactor other parts of core
that should be handled in a state machine e.g. pages-query-runner.js

This PR also reinforced the need for us to implement
[tracing](https://github.com/jaegertracing/jaeger) in core / plugins
as that'd make it far far easier to understand what's happening and
when.

* Document state machine and remove extraneous Chokidar states

* Remove console.log

* [json-loader] Only log file events if we're past bootstrap (gatsbyjs#4826)

* Don't emit new file node until previous is finished processing

This is an experiment to use
[xstate](http://davidkpiano.github.io/xstate/docs/#/) to setup state
machines to better handle complex state changes as we sometimes have.

Ideally this happens in core and then gatsby-source-filesystem
just has a simple queue and emits a new file node every time
the system returns to idle.

In a future refactor we'll do that plus refactor other parts of core
that should be handled in a state machine e.g. pages-query-runner.js

This PR also reinforced the need for us to implement
[tracing](https://github.com/jaegertracing/jaeger) in core / plugins
as that'd make it far far easier to understand what's happening and
when.

* Document state machine and remove extraneous Chokidar states

* Remove console.log

* Only log file events if we're past bootstrap

* [json-loader] dont recompile on data change - part 2 (gatsbyjs#4837)

* prevent adding duplicate redirects

* don't write new `redirects.json` if redirects didn't change

prevents webpack recompilation on data change

* [json-loader] develop - reading results from file improvments (gatsbyjs#4850)

* dont emit results for layouts

* [develop] store query results in memory, read json data from file only if we don't have it stored yet (we didn't run this query, but results are cached)

* Add query prioritization based on what page(s) user(s) are on

Query running is sadly not very ludicrous right now on gatsbyjs.org —
not sure why — each markdown file change causes ~20 queries to run but
even with prioritizing the active page's query, it's still ~2 seconds
before the page updates.

This sort of thing will be much easier to debug with tracing support.

* Add initial forward slash

* Actually this is how we add back the initial forward slash
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

Successfully merging this pull request may close these issues.

4 participants