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

Add Codebase Overview #7828

Merged
merged 3 commits into from
Sep 29, 2016
Merged

Add Codebase Overview #7828

merged 3 commits into from
Sep 29, 2016

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Sep 29, 2016

I'm not sure it's worth writing another Internals Guide because I feel like this covers most questions and gives good starting points for the stack reconciler. If not, I can do it though!

var setInnerHTML = require('setInnerHTML');
```

Haste was originally developed for giant apps like Facebook. It's easy to move files to different folders and iports them without worrying about relative paths. The fuzzy file search in any editor always takes you to the correct place thanks to globally unique names.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probable typo with iports here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, thx

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for writing such a detailed guide 👍

@sebmarkbage
Copy link
Collaborator

sebmarkbage commented Sep 29, 2016

Might be worth while explaining the /shared/ file structure? The principle is that /shared/ folder contains stuff that is shared by content in other folders at that same level.

If we were to switch to CommonJS resolution that would mean that a file can only require things at the same level or below (./x or ./x/y) OR it can only step upwards to a shared folder (../shared/x or ../../../shared/x) but never ../x or ../../renderer/dom/x.

@gaearon
Copy link
Collaborator Author

gaearon commented Sep 29, 2016

Going to get it in and do a separate pass about shared and few other things.

@gaearon gaearon merged commit 4f345e0 into facebook:master Sep 29, 2016
@gaearon gaearon deleted the codebase-overview branch September 29, 2016 10:34
gaearon added a commit that referenced this pull request Sep 29, 2016
* Add Codebase Overview

* Update codebase-overview.md

* Update codebase-overview.md
@ghost
Copy link

ghost commented Sep 29, 2016

I think some links are broken (those that have /react/contributing/.html pattern)

@gaearon
Copy link
Collaborator Author

gaearon commented Sep 29, 2016

The way our docs website is set up, links are broken on GitHub but they work on the website:
https://facebook.github.io/react/contributing/codebase-overview.html

I know it’s confusing 😞

var setInnerHTML = require('setInnerHTML');
```

Haste was originally developed for giant apps like Facebook. It's easy to move files to different folders and import them without worrying about relative paths. The fuzzy file search in any editor always takes you to the correct place thanks to globally unique names.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, haste doesn't have the constraint that filename needs to be unique nor that it matches the name that you require. Haste only looks at the @providesModule token in the first comment of the file.

Most people on fb codebase use the same filename because why invent a new name and it indeeds make it easier to search files by name.

Now, React in addition to haste has an export step to npm that takes all the files, move them to the root directory and rewrites all the require('Module') to require('./Module') which uses the filename.

So in the React codebase, filenames must match the name of the @providesModule. React Native doesn't have the same step so you can name your files whatever you want for example.

Feel free to leave this comment as is, but I figured I would let you know what is exactly happening :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea I learned that since I first wrote this (as a draft) but couldn't figure out how to explain it without sounding even more confusing. Left it as is because it applies in our context.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, @cpojer was planning to try to fix this and make them match. fingers crossed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. It is gonna happen EOY or early H1 2017.

acusti pushed a commit to brandcast/react that referenced this pull request Mar 15, 2017
* Add Codebase Overview

* Update codebase-overview.md

* Update codebase-overview.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants