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

Named classes #729

Merged
1 commit merged into from
Oct 5, 2010
Merged

Named classes #729

1 commit merged into from
Oct 5, 2010

Conversation

zmthy
Copy link
Collaborator

@zmthy zmthy commented Sep 27, 2010

This should allow us to have classes with a name property that corresponds to the actual class name without causing the weird IE scoping error. It's listed here as a solution, but I haven't actually tested it, so it'd be swell if someone could give that a go.

Note that we could do this for all of our functions, but things would get overly complicated way too quickly with the extra closure that this solution uses, and that's not really in the spirit of Coffeescript.

@jashkenas
Copy link
Owner

Hey Tesco, this is a lovely patch, but as discussed on IRC, I don't think that it's necessary for modern browsers. Take a look at this stacktrace, using an uncompressed version of the current master compiler, and Google Chrome.

image

Closing as a wontfix, because of the verbosity of the generated code.

@jashkenas
Copy link
Owner

Reopening the ticket...

Tesco, you currently have this as the compiled output:

SomeNode = (function() {
  function SomeNode() {
    ...
  }
  return SomeNode;
})();

What do you think about compiling to this format instead?

SomeNode = (function() {
  return function SomeNode() {
    ...
  }
})();

@zmthy
Copy link
Collaborator Author

zmthy commented Oct 5, 2010

Err... what? Is that even valid code?

@jashkenas
Copy link
Owner

Sorry about that ... edited. Pop on in to #coffeescript if you'd like to get this merged now...

@zmthy
Copy link
Collaborator Author

zmthy commented Oct 5, 2010

Merged with the latest patches, and updated to not take up any more lines than non-named classed did.

This pull request was closed.
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.

2 participants