This repository has been archived by the owner on May 3, 2024. It is now read-only.
docs(recipes/making-an-api-call): update #387
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
documentation
📊 Bundle Size Report
|
JAdshead
force-pushed
the
docs/working-with-data
branch
from
272a6d0
to
e47d818
Compare
10xLaCroixDrinker
suggested changes
Dec 3, 2020
docs/recipes/Module-Composition.md
Outdated
| const ParentModule = () => ( | ||
| <div> | ||
| <h1>I am the parent module</h1> | ||
| <RenderModule moduleName="ChildModule" props={{ greeting: 'hello' }} />; |
There was a problem hiding this comment.
Don't we want to encourage the convention of kebab-case for module names?
docs/recipes/Module-Composition.md
Outdated
| ``` | ||
|
|
||
| To use RenderModule we need to ensure that the module bundle has been correctly | ||
| loaded into our client or server before it can be rendered. We can do this by |
There was a problem hiding this comment.
All server bundles are already stored in memory. On the server it's only the module's data we would need to load.
infoxicator
reviewed
Dec 4, 2020
Co-authored-by: Ruben Casas <ruben@infoxication.net>
infoxicator
previously approved these changes
Dec 4, 2020
docs/recipes/Making-An-Api-Call.md
Outdated
| } | ||
| }; | ||
| fetchBooks(); | ||
| }, [id]); |
There was a problem hiding this comment.
Where is id coming from?
Co-authored-by: Mike Tobia <Francois-Esquire@users.noreply.github.com>
Francois-Esquire
previously approved these changes
Dec 8, 2020
JamesSingleton
previously approved these changes
Dec 8, 2020
nellyk
reviewed
Dec 9, 2020
nellyk
reviewed
Dec 9, 2020
| }; | ||
|
|
||
| Books.holocron = { | ||
| // Runs on both Server and Browser |
There was a problem hiding this comment.
Suggested change
| // Runs on both Server and Browser | |
| // Runs on both Server and Browser. Use global.BROWSER to alternate between the two |
There was a problem hiding this comment.
i think in this context that could be confusing
nellyk
reviewed
Dec 9, 2020
| }; | ||
| ``` | ||
|
|
||
| The modules reducer would handle those dispatched actions so the module would be able to retrieve the data from the Redux store. |
There was a problem hiding this comment.
Suggested change
| The modules reducer would handle those dispatched actions so the module would be able to retrieve the data from the Redux store. | |
| The modules reducer would handle those dispatched actions so the module would be able to retrieve the data from the Redux store. Depending on your case this value is not mandatory |
There was a problem hiding this comment.
to fetch this on the server and have the data propagate to the ui using the reducer would be required
nellyk
reviewed
Dec 9, 2020
| const { isLoading, data, error } = useFetchye('https://some-data-server.com/api/v1/books'); | ||
| const books = data && data.body; | ||
|
|
||
| if (isLoading) { |
There was a problem hiding this comment.
should we also advice using data.body.status===200
There was a problem hiding this comment.
we could add a check for data.ok but that is something i expect an engineer to handle, this is just a basic example of the fetchye api
Co-authored-by: Nelly Kiboi <nelly.j.kiboi@aexp.com>
JAdshead
dismissed stale reviews from JamesSingleton and Francois-Esquire
via
42e44c8
10xLaCroixDrinker
approved these changes
Dec 10, 2020
JamesSingleton
approved these changes
Dec 10, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Motivation and Context
How Has This Been Tested?
Types of Changes
Checklist:
What is the Impact to Developers Using One App?