-
Notifications
You must be signed in to change notification settings - Fork 49.6k
fix broken docs links #8163
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
fix broken docs links #8163
Conversation
Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours has expired. Before we can review or merge your code, we need you to email cla@fb.com with your details so we can update your status. |
For example, code shared between [`src/renderers/dom/client`](https://github.com/facebook/react/blob/87724bd87506325fcaf2648c70fc1f43411a87be/src/renderers/dom/client) and [`src/renderers/dom/server`](https://github.com/facebook/react/blob/87724bd87506325fcaf2648c70fc1f43411a87be/src/renderers/dom/server) lives in [`src/renderers/dom/shared`](https://github.com/facebook/react/tree/master/src/renderers/dom/shared). | ||
|
||
By the same logic, if [`src/renderers/dom/client`](https://github.com/facebook/react/tree/master/src/renderers/dom/client) needs to share a utility with something in [`src/renderers/native`](https://github.com/facebook/react/tree/master/src/renderers/native), this utility would be in [`src/renderers/shared`](https://github.com/facebook/react/tree/master/src/renderers/shared). | ||
By the same logic, if [`src/renderers/dom/client`](https://github.com/facebook/react/blob/87724bd87506325fcaf2648c70fc1f43411a87be/src/renderers/dom/client) needs to share a utility with something in [`src/renderers/native`](https://github.com/facebook/react/blob/87724bd87506325fcaf2648c70fc1f43411a87be/src/renderers/native), this utility would be in [`src/renderers/shared`](https://github.com/facebook/react/blob/87724bd87506325fcaf2648c70fc1f43411a87be/src/renderers/shared). |
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.
/src/renderers/native
and /src/renderers/shared
links exists in the master branch
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.
Just trying to follow a standard, all links inside the block should point to the same tree.
There is an exception to this rule. Sometimes we *do* need to share functionality between two groups of modules. In this case, we hoist the shared module up to a folder called `shared` inside the closest common ancestor folder of the modules that need to rely on it. | ||
|
||
For example, code shared between [`src/renderers/dom/client`](https://github.com/facebook/react/tree/master/src/renderers/dom/client) and [`src/renderers/dom/server`](https://github.com/facebook/react/tree/master/src/renderers/dom/server) lives in [`src/renderers/dom/shared`](https://github.com/facebook/react/tree/master/src/renderers/dom/shared). | ||
For example, code shared between [`src/renderers/dom/client`](https://github.com/facebook/react/blob/87724bd87506325fcaf2648c70fc1f43411a87be/src/renderers/dom/client) and [`src/renderers/dom/server`](https://github.com/facebook/react/blob/87724bd87506325fcaf2648c70fc1f43411a87be/src/renderers/dom/server) lives in [`src/renderers/dom/shared`](https://github.com/facebook/react/tree/master/src/renderers/dom/shared). |
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.
It seems like it should be fixed to the stack folder /src/renderers/dom/stack/client
and /src/renderers/dom/stack/server
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 thought about it, but as Fiber is still very recent and folder structure can still change, I think it's better to link a frozen tree. Aside from that, most other examples on this page also links to the same commit hash. I'm open for changes btw, as long as the link works 👍
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
It would be great if you could update all links to point to a frozen tree from today's master. We don't plan to change the structure much in the close future. |
@gaearon done |
Could you please revert changes to |
65ff00b
to
891ab32
Compare
Thanks! |
* fix broken links to outdated code * another broken links to outdated code * update hash commit & folder structure to current (cherry picked from commit b847226)
* fix broken links to outdated code * another broken links to outdated code * update hash commit & folder structure to current
Fiber changed the structure of
renderers/dom
folder and broke some docs links. As the structure may still change, I suppose it's better to rely on a commit hash for now.