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 support for typeof symbol === "symbol" #2242

Closed
wants to merge 1 commit into from

Conversation

jamiebuilds
Copy link
Contributor

Fixes #2241

@caitp
Copy link
Member

caitp commented Mar 8, 2015

I think you're going to need to add another assertion to the tests that use that fixture, but otherwise it looks p. good. Will merge when tests are fixed

@caitp
Copy link
Member

caitp commented Mar 8, 2015

actually, I guess you probably won't need to add an assertion, since it's testing that an error isn't thrown... I guess it's okay :>

@jamiebuilds
Copy link
Contributor Author

It does error if I remove the fix

@caitp
Copy link
Member

caitp commented Mar 8, 2015

yeah it's fine, we might want to do a followup to support the new futurehostile feature with it, which would call for some assertions --- but I think this is fine for now

@jamiebuilds
Copy link
Contributor Author

to support the new futurehostile feature with it,

I'm not sure I understand what that'd consist of.

@caitp caitp closed this in 7f7aac2 Mar 8, 2015
@jamiebuilds jamiebuilds deleted the typeof-symbol branch March 8, 2015 02:19
@caitp
Copy link
Member

caitp commented Mar 8, 2015

I'm not sure I understand what that'd consist of.

It just means making it a bit more complicated so that people authoring for ES3/5 environments can keep new features out of their code

@jamiebuilds
Copy link
Contributor Author

Should it selectively add "symbol" to the typeof list when this option is enabled?

@caitp
Copy link
Member

caitp commented Mar 8, 2015

something like that, I'm not sure exactly. @jugglinmike is the expert on it, but it's probably something we can hold back on until someone complains

@jugglinmike
Copy link
Member

I don't think the follow-up should use futurehostile because that option is intended to help people avoid authoring code that is difficult to upgrade to future versions of the language. This feature doesn't raise any future portability concerns; in this case, we just want to continue warning about "symbol" in pre-ES6 environments.

Doing this will require additional abstraction around the values array to key off the configured language version.

jugglinmike pushed a commit to jugglinmike/jshint that referenced this pull request Mar 15, 2015
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.

Invalid typeof value 'symbol'
3 participants