Skip to content

Add ArrayBuffer/DataView/Symbol/#toString. Improve Errors #332

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

Merged
merged 14 commits into from
Nov 18, 2018
Merged

Add ArrayBuffer/DataView/Symbol/#toString. Improve Errors #332

merged 14 commits into from
Nov 18, 2018

Conversation

MaxGraey
Copy link
Member

No description provided.

@@ -1,12 +1,11 @@
export class Error {

name: string = "Error";
message: string;
stack: string = ""; // TODO
stack: string | null = null; // TODO
Copy link
Member

Choose a reason for hiding this comment

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

Afaik this is never null in JS

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but it could be undefined according to this

Or not?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually Error.prototype.stack is not standard but all browsers support it

Copy link
Member

Choose a reason for hiding this comment

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

My initial though was that if it's guaranteed to be a string, even if empty, one can simply print it without having to check explicitly, which is about what you'd do in JS because it's effectively always there.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I'll revert this

@MaxGraey MaxGraey changed the title Add ArrayBuffer#toString & DataView#toString. Also minor improvments for Error Add ArrayBuffer/DataView/Symbol/#toString. Also minor improvments for Error Nov 12, 2018
}
return "Symbol()";
if (idToString !== null && idToString.has(id)) str = idToString.get(id);
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this should be the default case of the switch above

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, make sense

@MaxGraey MaxGraey changed the title Add ArrayBuffer/DataView/Symbol/#toString. Also minor improvments for Error Add ArrayBuffer/DataView/Symbol/#toString. Improvments Errors Nov 13, 2018
@MaxGraey MaxGraey changed the title Add ArrayBuffer/DataView/Symbol/#toString. Improvments Errors Add ArrayBuffer/DataView/Symbol/#toString. Improve Errors Nov 13, 2018
@dcodeIO dcodeIO merged commit 1928404 into AssemblyScript:master Nov 18, 2018
@dcodeIO
Copy link
Member

dcodeIO commented Nov 18, 2018

Thanks! :)

@chicoxyzzy
Copy link

What about Symbol.toStringTag?

@dcodeIO
Copy link
Member

dcodeIO commented Nov 18, 2018

That should be there.

@chicoxyzzy
Copy link

chicoxyzzy commented Nov 18, 2018

Yes, but I mean

class ArrayBuffer {
  ...
  get [Symbol.toStringTag]() { // should be added
    return 'ArrayBuffer';
  }
  ...
}

Object.prototype.toString.call(new ArrayBuffer()); // to make this work

@MaxGraey MaxGraey deleted the add-some-to-string branch November 18, 2018 11:10
@dcodeIO
Copy link
Member

dcodeIO commented Nov 18, 2018

While the symbols are there, these aren't really supported yet. [Symbol.something] syntax for properties isn't there yet, for example. The most prominent reason for having them defined already is that there are plans to implement iterators eventually, but it is unlikely that something like Function#call, as in your snippet, will be supported because it would imply that an untyped argument is provided, which isn't really AOT compatible, in turn rendering some of the built-in symbols like toStringTag obsolete.

@chicoxyzzy
Copy link

makes sense, thanks for the explanation

willemneal pushed a commit to willemneal/assemblyscript that referenced this pull request Dec 26, 2018
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.

3 participants