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

Replace "instanceof Array" in transformer with "[object Array]" comparison #3984

Closed
wants to merge 1 commit into from

Conversation

ide
Copy link
Contributor

@ide ide commented May 21, 2015

Testing with '[object Array]' is Object::toString.call element allows arrays from another JS context to be properly handled. The specific use case here is to support jest, which sets up JS contexts using Node/io.js's "vm" module. This approach works in ES3 environments in contrast with ES5's Array.isArray.

Array.isArray allows arrays from another JS context to be properly handled. The specific use case here is to support jest, which sets up JS contexts using Node/io.js's "vm" module.

The downside to this patch is that IE8 needs a polyfill for Array.isArray. If IE8 support for the in-browser transform (not the output of the coffeescript compiler) is important, it would be simple to include a polyfill with coffee-script.
@michaelficarra
Copy link
Collaborator

Sorry, we can't accept PRs that drop ES3 support.

@michaelficarra
Copy link
Collaborator

Though we could do a similar check, "[object Array]" is {}.toString.call element

@ide
Copy link
Contributor Author

ide commented May 21, 2015

I was hoping some kind of shim would be acceptable for environments that don't support Array.isArray natively, like

isArray = (value) ->
  if Array.isArray?
    Array.isArray value
  else
    '[object Array]' is Object::toString.call element

@ide ide changed the title Use "Array.isArray" instead of "instanceof Array" Replace "instanceof Array" in transformer with "[object Array]" comparison May 21, 2015
@ide
Copy link
Contributor Author

ide commented May 21, 2015

@michaelficarra: I updated the commit with your suggestion of checking with toString. Could you take a look please?

edit: I didn't realize that github doesn't update closed PRs. New PR at #3985. Sorry about that.

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.

2 participants