Taking the time to clean up the codebase #75
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
|
The backwards breaking changes here are the removal of custom errors. |
tbranyen
force-pushed
the
general-cleanup
branch
4 times, most recently
from
974ffc1
to
49293c2
Compare
I've wanted to take the time to clean up the codebase and ensure everything is consistent. Right now my refactor efforts are to use common nomenclature when talking about Virtual Tree Elements and DOM Nodes. This distinction is important in the codebase, and is currently different depending on the file you're in. Sometimes they're called entry, sometimes tree, descriptor, etc. I'm consolidating all the variable names to vTree and node. Another important refactor is to provide clean and consistent JSDoc comments above all functions public/private. I'm attempting to use `const` wherever possible to denote that a variable is never re-assigned. I'm refactoring branched code that required using `var` or `let` into functions. I will still use `let` for loops, since we are tight-loop critical and I don't like the overhead of functional looping here. There are parts of the codebase that are highly duplicative and I'm leveraging the createElement/createAttribute functions, exposed for the Babel transform, internally. In preparation for a 1.0 release, I want to make sure all the code makes sense and references variables consistently. This furthers that push by making it explicit that an object is either a DOM Node or a Virtual Tree Element/Attribute. This made it easier to re-arrange the file structure to better match what they do. For instance, before makeElement made a DOM Node and makeNode made a Virtual Tree Element. This didn't affect tests other than for import paths in the internal unit tests.
tbranyen
force-pushed
the
general-cleanup
branch
from
49293c2
to
ea1dacf
Compare
|
Since this PR is fairly significant, I'm going to merge as-is and continue refactoring each specific module individually. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I've wanted to take the time to clean up the codebase and ensure
everything is consistent. Right now my refactor efforts are to use
common nomenclature when talking about Virtual Tree Elements and DOM
Nodes. This distinction is important in the codebase, and is currently
different depending on the file you're in. Sometimes they're called
entry, sometimes tree, descriptor, etc. I'm consolidating all the
variable names to vTree and node.
Another important refactor is to provide clean and consistent JSDoc
comments above all functions public/private.
I'm attempting to use
constwherever possible to denote that avariable is never re-assigned. I'm refactoring branched code that
required using
varorletinto functions. I will still useletforloops, since we are tight-loop critical and I don't like the overhead of
functional looping here.
There are parts of the codebase that are highly duplicative and I'm
leveraging the createElement/createAttribute functions, exposed for the
Babel transform, internally.