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

Miscellaneous work #45

Merged
merged 1 commit into from Feb 8, 2016
Merged

Miscellaneous work #45

merged 1 commit into from Feb 8, 2016

Conversation

tbranyen
Copy link
Owner

@tbranyen tbranyen commented Feb 8, 2016

This is a collection of miscellaneous work I've been doing. Some stuff floating around in this PR:

  • Experimenting with webpack, although not sure it's a huge win since the dist is run through derequire
  • Added function names instead of leaving them anonymous
  • Swapped out for clearer Array#push instead of using Array#length and assigning directly
  • Fixed the bug where transitions were always recursively called into children when only attached and detached should
  • Better HTML markup handling, we conform better to the HTML spec, by removing whitespace between <html></html> and automatically add in missing <head></head> and <body></body>.

@jamesplease
Copy link

This file seems to be a placeholder

let allocatedLength = allocated.length;
let freeLength = free.length;
let reAlloc = allocated.slice(0, size - freeLength);
freeAll: function freeAllPool() {

Choose a reason for hiding this comment

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

Have you considered switching to

freeAll() {}

rather than trying to name the fn's yourself? In large codebases I find that manually naming things is one of the first things to go unmaintained. Over time I end up seeing:

{
  methodOne: function methodOne() {},
  methodTwo: function methodOne() {},
  methodThree: function methodOne() {}
}

which is understandable, 'cause it's so easy to miss!

You might know this already, but in an ES2015-compliant environment, even anonymous functions will be automatically named (spec here), but this isn't implemented in any environment that I know of atm. What is implemented is giving a name to a function defined with the shorthand syntax, like:

freeAll() {}

You can test this by running these quick tests in Chrome:

var testOne = {
  someProp: function() {}
};
testOne.someProp.name === ""; // true
var testTwo = {
  someProp() {}
};
testTwo.someProp.name === "someProp"; // true

Copy link
Owner Author

Choose a reason for hiding this comment

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

This is a good point. I should refactor this code. The reason why Pool was attached is that I was colliding with internal variables. Like var protect = [] and then function protect would collide. This is easily fixed by updating the variable names to reflect what they are.

@jugglinmike had shown me the new naming semantics, but you're right I don't think they are actually implemented anywhere.

@tbranyen tbranyen changed the title Misc work Miscellaneous work Feb 8, 2016
@tbranyen
Copy link
Owner Author

tbranyen commented Feb 8, 2016

Yup that file was added with the intention of migrating some code out of patches/process.js. I'll remove it for now since it's confusingly empty.

@tbranyen tbranyen force-pushed the misc-work branch 2 times, most recently from 1dba79b to 8ae5f26 Compare February 8, 2016 21:22
- Experimenting with webpack, although not sure it's a huge win since
  the dist is run through derequire
- Added function names instead of leaving them anonymous
- Swapped out for clearer Array#push instead of using Array#length and
  assigning directly
- Fixed the bug where transitions were always recursively called into
  children when only attached and detached should
- Better HTML markup handling, we conform better to the HTML spec, by
  removing whitespace between <html></html> and automatically add in
  missing <head></head> and <body></body>.
tbranyen added a commit that referenced this pull request Feb 8, 2016
@tbranyen tbranyen merged commit 1ed4c6c into master Feb 8, 2016
@tbranyen tbranyen deleted the misc-work branch February 8, 2016 21:27
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.

None yet

2 participants