-
Notifications
You must be signed in to change notification settings - Fork 58
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 RequireRemote#load to load external Ruby scripts from server #292
Merged
Merged
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
115dea3
Add JS::RequireRemote to load external Ruby scripts from the browser
ledsun acbf323
Stop extending setupProxy. Add Playwright routing instead.
ledsun 45755c4
Add a test case that repeats require_relative recursively.
ledsun b9db613
Add unit tests for JS::RequireRemote::URLResolver
ledsun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
require "singleton" | ||
require "js" | ||
require_relative "./require_remote/url_resolver" | ||
require_relative "./require_remote/evaluator" | ||
|
||
module JS | ||
# This class is used to load remote Ruby scripts. | ||
# | ||
# == Example | ||
# | ||
# require 'js/require_remote' | ||
# JS::RequireRemote.instance.load("foo") | ||
# | ||
# This class is intended to be used to replace Kernel#require_relative. | ||
# | ||
# == Example | ||
# | ||
# require 'js/require_remote' | ||
# module Kernel | ||
# def require_relative(path) = JS::RequireRemote.instance.load(path) | ||
# end | ||
# | ||
# If you want to load the bundled gem | ||
# | ||
# == Example | ||
# | ||
# require 'js/require_remote' | ||
# module Kernel | ||
# alias original_require_relative require_relative | ||
# | ||
# def require_relative(path) | ||
# caller_path = caller_locations(1, 1).first.absolute_path || '' | ||
# dir = File.dirname(caller_path) | ||
# file = File.absolute_path(path, dir) | ||
# | ||
# original_require_relative(file) | ||
# rescue LoadError | ||
# JS::RequireRemote.instance.load(path) | ||
# end | ||
# end | ||
# | ||
class RequireRemote | ||
include Singleton | ||
|
||
def initialize | ||
base_url = JS.global[:URL].new(JS.global[:location][:href]) | ||
@resolver = URLResolver.new(base_url) | ||
@evaluator = Evaluator.new | ||
end | ||
|
||
# Load the given feature from remote. | ||
def load(relative_feature) | ||
location = @resolver.get_location(relative_feature) | ||
|
||
# Do not load the same URL twice. | ||
return false if @evaluator.evaluated?(location.url[:href].to_s) | ||
|
||
response = JS.global.fetch(location.url).await | ||
unless response[:status].to_i == 200 | ||
raise LoadError.new "cannot load such url -- #{response[:status]} #{location.url}" | ||
end | ||
|
||
# The fetch API may have responded to a redirect response | ||
# and fetched the script from a different URL than the original URL. | ||
# Retrieve the final URL again from the response object. | ||
final_url = response[:url].to_s | ||
|
||
# Do not evaluate the same URL twice. | ||
return false if @evaluator.evaluated?(final_url) | ||
|
||
code = response.text().await.to_s | ||
|
||
evaluate(code, location.filename, final_url) | ||
end | ||
|
||
private | ||
|
||
def evaluate(code, filename, final_url) | ||
@resolver.push(final_url) | ||
@evaluator.evaluate(code, filename, final_url) | ||
@resolver.pop | ||
true | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
module JS | ||
class RequireRemote | ||
# Execute the body of the response and record the URL. | ||
class Evaluator | ||
def evaluate(code, filename, final_url) | ||
Kernel.eval(code, ::Object::TOPLEVEL_BINDING, filename) | ||
$LOADED_FEATURES << final_url | ||
end | ||
|
||
def evaluated?(url) | ||
$LOADED_FEATURES.include?(url) | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
module JS | ||
class RequireRemote | ||
ScriptLocation = Data.define(:url, :filename) | ||
|
||
# When require_relative is called within a running Ruby script, | ||
# the URL is resolved from a relative file path based on the URL of the running Ruby script. | ||
# It uses a stack to store URLs of running Ruby Script. | ||
# Push the URL onto the stack before executing the new script. | ||
# Then pop it when the script has finished executing. | ||
class URLResolver | ||
def initialize(base_url) | ||
@url_stack = [base_url] | ||
end | ||
|
||
def get_location(relative_feature) | ||
filename = filename_from(relative_feature) | ||
url = resolve(filename) | ||
ScriptLocation.new(url, filename) | ||
end | ||
|
||
def push(url) | ||
@url_stack.push url | ||
end | ||
|
||
def pop() | ||
@url_stack.pop | ||
end | ||
|
||
private | ||
|
||
def filename_from(relative_feature) | ||
if relative_feature.end_with?(".rb") | ||
relative_feature | ||
else | ||
"#{relative_feature}.rb" | ||
end | ||
end | ||
|
||
# Return a URL object of JavaScript. | ||
def resolve(relative_filepath) | ||
JS.global[:URL].new relative_filepath, @url_stack.last | ||
end | ||
end | ||
end | ||
end |
5 changes: 5 additions & 0 deletions
5
packages/npm-packages/ruby-wasm-wasi/example/require_relative/greeting.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
class Greeting | ||
def say | ||
puts "Hello, world!" | ||
end | ||
end |
31 changes: 31 additions & 0 deletions
31
packages/npm-packages/ruby-wasm-wasi/example/require_relative/index.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
<html> | ||
<script src="https://cdn.jsdelivr.net/npm/@ruby/[email protected]/dist/browser.script.iife.js"></script> | ||
<script type="text/ruby" data-eval="async"> | ||
# Patch require_relative to load from remote | ||
require 'js/require_remote' | ||
|
||
module Kernel | ||
alias original_require_relative require_relative | ||
|
||
# The require_relative may be used in the embedded Gem. | ||
# First try to load from the built-in filesystem, and if that fails, | ||
# load from the URL. | ||
def require_relative(path) | ||
caller_path = caller_locations(1, 1).first.absolute_path || '' | ||
dir = File.dirname(caller_path) | ||
file = File.absolute_path(path, dir) | ||
|
||
original_require_relative(file) | ||
rescue LoadError | ||
JS::RequireRemote.instance.load(path) | ||
end | ||
end | ||
|
||
# The above patch does not break the original require_relative | ||
require 'csv' | ||
csv = CSV.new "foo\nbar\n" | ||
|
||
# Load the main script | ||
require_relative 'main' | ||
</script> | ||
</html> |
3 changes: 3 additions & 0 deletions
3
packages/npm-packages/ruby-wasm-wasi/example/require_relative/main.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
require_relative "greeting" | ||
|
||
Greeting.new.say |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3 changes: 3 additions & 0 deletions
3
packages/npm-packages/ruby-wasm-wasi/test-e2e/integrations/fixtures/error_on_load_twice.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
raise "load twice" if defined?(ALREADY_LOADED) | ||
|
||
ALREADY_LOADED = true |
1 change: 1 addition & 0 deletions
1
packages/npm-packages/ruby-wasm-wasi/test-e2e/integrations/fixtures/recursive_require.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
require_relative "./recursive_require/a.rb" |
1 change: 1 addition & 0 deletions
1
packages/npm-packages/ruby-wasm-wasi/test-e2e/integrations/fixtures/recursive_require/a.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
require_relative "./b.rb" |
7 changes: 7 additions & 0 deletions
7
packages/npm-packages/ruby-wasm-wasi/test-e2e/integrations/fixtures/recursive_require/b.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
module RecursiveRequire | ||
class B | ||
def message | ||
"Hello from RecursiveRequire::B" | ||
end | ||
end | ||
end |
119 changes: 119 additions & 0 deletions
119
packages/npm-packages/ruby-wasm-wasi/test-e2e/integrations/js-require-remote.spec.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
import fs from "fs"; | ||
import path from "path"; | ||
import { test, expect } from "@playwright/test"; | ||
import { | ||
setupDebugLog, | ||
setupProxy, | ||
setupUncaughtExceptionRejection, | ||
expectUncaughtException, | ||
resolveBinding, | ||
} from "../support"; | ||
|
||
if (!process.env.RUBY_NPM_PACKAGE_ROOT) { | ||
test.skip("skip", () => {}); | ||
} else { | ||
test.beforeEach(async ({ context, page }) => { | ||
setupDebugLog(context); | ||
setupProxy(context); | ||
|
||
const fixturesPattern = /fixtures\/(.+)/; | ||
context.route(fixturesPattern, (route) => { | ||
const subPath = route.request().url().match(fixturesPattern)[1]; | ||
const mockedPath = path.join("./test-e2e/integrations/fixtures", subPath); | ||
|
||
route.fulfill({ | ||
path: mockedPath, | ||
}); | ||
}); | ||
|
||
context.route(/not_found/, (route) => { | ||
route.fulfill({ | ||
status: 404, | ||
}); | ||
}); | ||
|
||
setupUncaughtExceptionRejection(page); | ||
}); | ||
|
||
test.describe("JS::RequireRemote#load", () => { | ||
test("JS::RequireRemote#load returns true", async ({ page }) => { | ||
const resolve = await resolveBinding(page, "checkResolved"); | ||
await page.goto( | ||
"https://cdn.jsdelivr.net/npm/@ruby/head-wasm-wasi@latest/dist/", | ||
); | ||
await page.setContent(` | ||
<script src="browser.script.iife.js"></script> | ||
<script type="text/ruby" data-eval="async"> | ||
require 'js/require_remote' | ||
JS.global.checkResolved JS::RequireRemote.instance.load 'fixtures/error_on_load_twice' | ||
</script> | ||
`); | ||
|
||
expect(await resolve()).toBe(true); | ||
}); | ||
|
||
test("JS::RequireRemote#load returns false when same gem is loaded twice", async ({ | ||
page, | ||
}) => { | ||
const resolve = await resolveBinding(page, "checkResolved"); | ||
await page.goto( | ||
"https://cdn.jsdelivr.net/npm/@ruby/head-wasm-wasi@latest/dist/", | ||
); | ||
await page.setContent(` | ||
<script src="browser.script.iife.js"></script> | ||
<script type="text/ruby" data-eval="async"> | ||
require 'js/require_remote' | ||
JS::RequireRemote.instance.load 'fixtures/error_on_load_twice' | ||
JS.global.checkResolved JS::RequireRemote.instance.load 'fixtures/error_on_load_twice' | ||
</script> | ||
`); | ||
|
||
expect(await resolve()).toBe(false); | ||
}); | ||
|
||
test("JS::RequireRemote#load throws error when gem is not found", async ({ | ||
page, | ||
}) => { | ||
expectUncaughtException(page); | ||
|
||
// Opens the URL that will be used as the basis for determining the relative URL. | ||
await page.goto( | ||
"https://cdn.jsdelivr.net/npm/@ruby/head-wasm-wasi@latest/dist/", | ||
); | ||
await page.setContent(` | ||
<script src="browser.script.iife.js"> | ||
</script> | ||
<script type="text/ruby" data-eval="async"> | ||
require 'js/require_remote' | ||
JS::RequireRemote.instance.load 'not_found' | ||
</script> | ||
`); | ||
|
||
const error = await page.waitForEvent("pageerror"); | ||
expect(error.message).toMatch(/cannot load such url -- .+\/not_found.rb/); | ||
}); | ||
|
||
test("JS::RequireRemote#load recursively loads dependencies", async ({ | ||
page, | ||
}) => { | ||
const resolve = await resolveBinding(page, "checkResolved"); | ||
await page.goto( | ||
"https://cdn.jsdelivr.net/npm/@ruby/head-wasm-wasi@latest/dist/", | ||
); | ||
await page.setContent(` | ||
<script src="browser.script.iife.js"></script> | ||
<script type="text/ruby" data-eval="async"> | ||
require 'js/require_remote' | ||
module Kernel | ||
def require_relative(path) = JS::RequireRemote.instance.load(path) | ||
end | ||
|
||
require_relative 'fixtures/recursive_require' | ||
JS.global.checkResolved RecursiveRequire::B.new.message | ||
</script> | ||
`); | ||
|
||
expect(await resolve()).toBe("Hello from RecursiveRequire::B"); | ||
}); | ||
}); | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to carefully handle relative path -> absolute path resolution here. IIUC JavaScript's
new URL(path, base)
does not behave as we expect forKernel#require_relative
(and alsoFile.expand_path
).Here is an example:
path, base
File.expand_path(path, base)
new URL(path, "https://example.com" + base)
/lib/b.rb
https://example.com/b.rb
/lib/b.rb
https://example.com/lib/b.rb
I think the test suite of
File.expand_path
would be helpful for you to see what should be cared https://github.com/ruby/ruby/blob/master/spec/ruby/core/file/expand_path_spec.rbThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this behavior of the JavaScript's URL constructor is as expected.
In most cases, I assume that the first value for
base
is the URL of an HTML file, such as http://exapmle.com/index.html, or a URL such as http://exapmle.com/index without the extension.Also, once the ruby script is loaded, the
base
value will be the URL of the ruby script, such as http://exapmle.com/a.rb.So we want the string after the last
/
in thebase
value to be ignored.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ok. I misunderstood the second argument. If the second argument is always a URL to a file, then it makes sense to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyway, unit tests covering this area would be preferable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wrote unit testsfor JS::RequireRemote::URLResolver.
Please let me know if there are any tests that you think we should add.