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

New definitions of "H", "P", "p" and "∞", supposed new Features ASYNC_FUNCTION, ITERATOR and JAPANESE_INFINITY #46

Merged
merged 44 commits into from
Aug 2, 2024

Conversation

Clayblockunova
Copy link
Contributor

@Clayblockunova Clayblockunova commented Jul 5, 2024

idk how hard adding new features by myself is, but I have some new definitions which rely on introducing new features (ITERATOR and JAPANESE_INFINITY).

ITERATOR is for new browsers which have iterator methods, and JAPANESE_INFINITY is for wider usage of Infinity.toLocaleString() in getting "∞" (IE11 on win7 Japanese Infinity string is "+∞").

src/lib/definitions.js Outdated Show resolved Hide resolved
src/lib/definitions.js Outdated Show resolved Hide resolved
src/lib/definitions.js Outdated Show resolved Hide resolved
src/lib/definitions.js Outdated Show resolved Hide resolved
src/lib/definitions.js Outdated Show resolved Hide resolved
@Clayblockunova
Copy link
Contributor Author

just added "p" definition.

@fasttime
Copy link
Owner

fasttime commented Jul 6, 2024

Currently, it's not trivial to add a new feature, I'll see if I can write some instructions. We need the features before we can add the new definitions.

@Clayblockunova
Copy link
Contributor Author

when will the instructions be finished? how will it be delivered?

@fasttime
Copy link
Owner

fasttime commented Jul 8, 2024

I should be able do it in the next days, I would like to include links to some example commits for reference. Maybe this should be a markdown page in the repo or a wiki page, what do you think?

@Clayblockunova
Copy link
Contributor Author

I think wiki page is better. And, those existing Markdown pages for API and features can be also turned into wiki pages too.

@Clayblockunova
Copy link
Contributor Author

sorry. I found that wiki was built days before but only have generic contribution guideline. when will you have specific instruction about creating new features?

@fasttime
Copy link
Owner

I'm working on this.

@fasttime
Copy link
Owner

I've added feature ITERATOR_HELPER and created instructions to add a feature here: https://github.com/fasttime/JScrewIt/wiki/Contributing#adding-a-feature. Do you think that's clear enough?

@Clayblockunova
Copy link
Contributor Author

mostly clear, but IDK how empty function can be used as simulation of Iterator. can new object type be defined by simply defining its constructor function?
additionally, for JAPANESE_INFINITY , idk can Infinity.toLocaleString("ja") be simply rewritten. if not, how to simulate?

@Clayblockunova Clayblockunova changed the title New definitions of "H" and "∞", supposed new Features ITERATOR and JAPANESE_INFINITY New definitions of "H", "P" and "∞", supposed new Features ASYNC_FUNCTION, ITERATOR and JAPANESE_INFINITY Jul 14, 2024
@Clayblockunova
Copy link
Contributor Author

btw, I just added a new definition of "P" for Node.js 7.6-15, which relies on a new feature: ASYNC_FUNCTION. this feature is syntax-related and idk how to simulate.

@Clayblockunova Clayblockunova changed the title New definitions of "H", "P" and "∞", supposed new Features ASYNC_FUNCTION, ITERATOR and JAPANESE_INFINITY New definitions of "H", "P", "p" and "∞", supposed new Features ASYNC_FUNCTION, ITERATOR and JAPANESE_INFINITY Jul 14, 2024
@fasttime
Copy link
Owner

mostly clear, but IDK how empty function can be used as simulation of Iterator. can new object type be defined by simply defining its constructor function?

That's fine. The emulated objects are just for testing, they are not expected to conform to the specification. For ITERATOR_HELPER, the only interesting property is the fact that iterator helper objects have a string representation like [object Iterator Helper]. Feel free to update the wiki when it's not clear.

additionally, for JAPANESE_INFINITY , idk can Infinity.toLocaleString("ja") be simply rewritten. if not, how to simulate?

You could have a look at how LOCALE_INFINITY is emulated. The main difference would be that the function inside registerNumberToLocaleStringAdapter will accept an argument locale, and if locale is 'ja' and this is Infinity, then the function will return '+∞'.

btw, I just added a new definition of "P" for Node.js 7.6-15, which relies on a new feature: ASYNC_FUNCTION. this feature is syntax-related and idk how to simulate.

It's necessary to overwrite the Function constructor. I did something like that for feature ARROW. I can have a look.

src/lib/definitions.js Show resolved Hide resolved
src/lib/definitions.js Outdated Show resolved Hide resolved
@Clayblockunova
Copy link
Contributor Author

For simulation of ARROW, you created a function for transforming arrow function to a normal one. should I do anything similar for simulation of ASYNC_FUNCTION?

src/lib/definitions.js Outdated Show resolved Hide resolved
src/lib/features.js Outdated Show resolved Hide resolved
src/lib/features.js Outdated Show resolved Hide resolved
src/lib/features.js Outdated Show resolved Hide resolved
@fasttime
Copy link
Owner

For simulation of ARROW, you created a function for transforming arrow function to a normal one. should I do anything similar for simulation of ASYNC_FUNCTION?

Feel free 👍

@Clayblockunova
Copy link
Contributor Author

sorry, but that's the best I can do.

Copy link
Owner

@fasttime fasttime left a comment

Choose a reason for hiding this comment

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

Thanks! That's not bad all 👍I found some time to review this yesterday and left some suggestions.

src/lib/features.js Outdated Show resolved Hide resolved
src/lib/features.js Show resolved Hide resolved
test/helpers/feature-emulation.helpers.js Outdated Show resolved Hide resolved
test/helpers/feature-emulation.helpers.js Outdated Show resolved Hide resolved
test/helpers/feature-emulation.helpers.js Outdated Show resolved Hide resolved
test/helpers/feature-emulation.helpers.js Outdated Show resolved Hide resolved
test/helpers/feature-emulation.helpers.js Outdated Show resolved Hide resolved
Copy link
Owner

@fasttime fasttime left a comment

Choose a reason for hiding this comment

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

Almost there.

test/helpers/feature-emulation.helpers.js Outdated Show resolved Hide resolved
test/helpers/feature-emulation.helpers.js Outdated Show resolved Hide resolved
test/helpers/feature-emulation.helpers.js Show resolved Hide resolved
@Clayblockunova
Copy link
Contributor Author

Still anything to do? or can be merged now?

Copy link
Owner

@fasttime fasttime left a comment

Choose a reason for hiding this comment

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

Sorry, was just testing browser compatibility. Left some comments.

src/lib/features.js Outdated Show resolved Hide resolved
src/lib/features.js Outdated Show resolved Hide resolved
src/lib/features.js Show resolved Hide resolved
Copy link
Owner

@fasttime fasttime left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM.

@fasttime fasttime merged commit 181b4fd into fasttime:main Aug 2, 2024
55 checks passed
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