Skip to content

Ability to call overridden functions from the std library #1216

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 7 commits into from
Jun 27, 2023

Conversation

flimzy
Copy link
Member

@flimzy flimzy commented Jun 26, 2023

Rebase of #798, and applied change suggested in #798 (comment), and a couple cleanups.

r-l-x and others added 3 commits June 26, 2023 14:20
Before this commit, every overriden standard library function was renamed
'_'. Now, if there is the following directive in the function comment:
"//gopherjs:keep_overridden", they are prefixed with "_gopherjs_overridden_"
so that they can be called from the overriding function. This is typically
useful to use the standard library implementation as a fallback.
Before this commit, TestOnePassCutoff was skipped because a 'Maximum
call stack size exceeded' message may occur.  Now we use
gopherjs:keep_overridden to try the original test, and skip only if the
original test failed.
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@flimzy
Copy link
Member Author

flimzy commented Jun 26, 2023

One last bikeshed on this one... Now that we've settled on gopherjs:keep-original, would it make sense to rename the original function to match? i.e. in place of:

_gopherjs_overridden_TestOnePassCutoff(t)

Do we want this?

_gopherjs_original_TestOnePassCutoff(t)

@github-actions

This comment has been minimized.

@nevkontakte
Copy link
Member

Do we want this?

_gopherjs_original_TestOnePassCutoff(t)

I don't feel too strongly, but seems like a good idea.

@github-actions
Copy link

Reference app: jQuery TodoMVC (acf500a6c32a83d8c4582d967b09a65febf0e120)

BRANCH ORIGINAL MINIFIED COMPRESSED (GZIP)
Pull request (keep_overridden) 2,948,040 bytes 1,934,883 bytes 382,916 bytes
Target branch (master) 0.00% increase (0 bytes) 0.00% increase (0 bytes) 0.00% increase (0 bytes)

#outputSize

@flimzy
Copy link
Member Author

flimzy commented Jun 27, 2023

Thanks @r-l-x for creating this original PR. I'm glad to see it finally merged!

@flimzy flimzy merged commit 698d025 into master Jun 27, 2023
@flimzy flimzy deleted the keep_overridden branch June 27, 2023 07:55
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