Skip to content

fix(jupyter): keep this around#20789

Merged
bartlomieju merged 2 commits intodenoland:mainfrom
rgbkrk:dont-lose-this
Oct 4, 2023
Merged

fix(jupyter): keep this around#20789
bartlomieju merged 2 commits intodenoland:mainfrom
rgbkrk:dont-lose-this

Conversation

@rgbkrk
Copy link
Copy Markdown
Contributor

@rgbkrk rgbkrk commented Oct 4, 2023

This fixes #20767.

image

We were losing this and then when an exception was happening, it didn't show up in the output because we weren't bubbling up exceptions from within a user defined function for displaying. I thought about doing a .call(object) but didn't want to get in the way of a bound this that a user or library was already putting on the function.

Copy link
Copy Markdown
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

Nice fix, thanks Kyle!

@rgbkrk
Copy link
Copy Markdown
Contributor Author

rgbkrk commented Oct 4, 2023

Rebased against main and tests are passing if you want to merge it now.

@bartlomieju bartlomieju merged commit cbddf57 into denoland:main Oct 4, 2023
@rgbkrk rgbkrk deleted the dont-lose-this branch October 4, 2023 21:25
bartlomieju pushed a commit that referenced this pull request Oct 12, 2023
This fixes #20767.

We were losing `this` and then when an exception was happening, it
didn't show up in the output because we weren't bubbling up exceptions
from within a user defined function for displaying. I thought about
doing a `.call(object)` but didn't want to get in the way of a bound
`this` that a user or library was already putting on the function.
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.

[jupyter] Symbol.for("Jupyter.display") not detected on classes

2 participants