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 a generic implementation of Kernel#caller and #warn(uplevel:) #2065

Merged
merged 5 commits into from
Jan 26, 2020

Conversation

elia
Copy link
Member

@elia elia commented Jan 17, 2020

Along with the recent changes to the node stacktrace this PR should be making the development experience both of opal itself and of opal apps much better.

Fixes #2068

@elia elia self-assigned this Jan 17, 2020
@elia elia added corelib feature nodejs source-maps usability Stuff that improves the life of the developer labels Jan 18, 2020
@elia elia force-pushed the elia/kernel-caller branch 4 times, most recently from b1cf493 to d86c92a Compare January 24, 2020 22:30
@elia elia marked this pull request as ready for review January 25, 2020 15:18
Copy link
Contributor

@iliabylich iliabylich left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@@ -92,8 +92,13 @@ def at_exit(&block)

# Opal does not support #caller, but we stub it as an empty array to not
Copy link
Contributor

Choose a reason for hiding this comment

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

✂️

if (path.endsWith('/index.js')) {
return {
url: './index.map',
map: #{builder.source_map.to_json.to_json}
Copy link
Contributor

Choose a reason for hiding this comment

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

can the second to_json be replaced with inspect?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not 100% sure the escaping is the same between the two… I since moved the generation of the values to be written to the top of the method, so the two to_json don't appear one after the other anymore.

Move all the file writing to the Ruby side and create a dedicated
temporary folder to hold the code. Add a version of the already
existing code for sourcemapped stack-traces that works on the browser
(via Browserify).
This implementation should be good enough for browsers and node.
@elia elia merged commit 216fa0c into master Jan 26, 2020
@elia elia deleted the elia/kernel-caller branch January 26, 2020 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
corelib feature nodejs source-maps usability Stuff that improves the life of the developer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Caller doesn't work
2 participants