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

type stability and compile time improvements #480

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 25 additions & 28 deletions src/asset.jl
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ specified if nonstandard extensions are in use.
"""
struct Asset
filetype::String
name::Union{Nothing, String}
name::String
url::String
end

Asset(name, url) = Asset(getextension(url), name, url)
Asset(url) = Asset(nothing, url)
Asset(url) = Asset("", url)
Asset(x::Pair) = Asset(string(x[1]), x[2])
Asset(x::Asset) = x
function Asset(spec::Dict)
Expand All @@ -42,23 +42,21 @@ function Asset(spec::Dict)
error("Invalid Asset dict specification: missing key \"url\".")
end
filetype = get(spec, "type", getextension(url))
name = get(spec, "name", nothing)
name = get(spec, "name", "")

return Asset(filetype, name, url)
end

function JSON.lower(asset::Asset)
Dict(
"type" => asset.filetype,
"url" => dep2url(asset.url),
"name" => asset.name,
)
(type=asset.filetype, url=dep2url(asset.url), name=asset.name,)
end


function tojs(asset::Asset)
return js"WebIO.importResource($(JSON.lower(asset)))"
la = JSON.lower(asset)
return _assettojs(la)
end
_assettojs(la) = js"WebIO.importResource($la)"

"""
Sync(assets...)
Expand All @@ -79,12 +77,12 @@ julia> WebIO.Sync(Asset("foo.js"), "bar" => "bar.js")
Sync(Asset[Asset("js", nothing, "foo.js"), Asset("js", "bar", "bar.js")])
```
"""
struct Sync
# This is untyped because we can't define a union type that includes all of
# Asset, Sync, and Async that is then used within Sync and Async.
imports::Array{Any}

Sync(imports::Array) = new([ensure_asset(asset) for asset in imports])
struct Sync{A<:Array}
imports::A
Sync(imports::A) where A<:Array = begin
assets = [ensure_asset(asset) for asset in imports]
new{typeof(assets)}(assets)
end
Comment on lines +80 to +85
Copy link
Member

Choose a reason for hiding this comment

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

This feels a bit awkward to me (to have a type parameter that's an array). Can you benchmark the changes?

Copy link
Author

Choose a reason for hiding this comment

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

This makes them type stable to access, Array just limits A to only be an Array. A is just a regular type parameter.

end
Sync(assets...) = Sync([assets...])

Expand All @@ -99,32 +97,31 @@ constructor for an [`Asset`](@ref), or a [`Sync`](@ref) or [`Async`](@ref).

If the imports need to be imported sequentially, use [`Sync`](@ref) instead.
"""
struct Async
# See comment about (lack of) typing in Sync above.
imports::Array{Any}
struct Async{A<:Array}
imports::A

Async(imports::Array) = new([ensure_asset(asset) for asset in imports])
function Async(imports::A) where A<:Array
assets = [ensure_asset(asset) for asset in imports]
new{typeof(assets)}(assets)
end
end
Async(assets...) = Async([assets...])

# This allows js"await $(Async([....]))" on a tree of assets!
function tojs(asset::Union{Async,Sync})
return js"WebIO.importBlock($(lowerassets(asset)))"
lowered = lowerassets(asset)
return _synctojs(lowered)
end
# Function barrier
_synctojs(la) = js"WebIO.importBlock($la)"

# The output of lowerassets is initially sent with the Scope
# this should trigger loading of the assets before onmount callbacks
lowerassets(x) = JSON.lower(Asset(x))
lowerassets(x::Asset) = JSON.lower(x)
lowerassets(x::Async) = Dict(
"type" => "async_block",
"data" => map(lowerassets, x.imports),
)
lowerassets(x::Async) = (type="async_block", data=map(lowerassets, x.imports))
lowerassets(x::AbstractArray) = lowerassets(Async(x))
lowerassets(x::Sync) = Dict(
"type"=>"sync_block",
"data" => map(lowerassets, x.imports),
)
lowerassets(x::Sync) = (type="sync_block", data=map(lowerassets, x.imports))

"""
ensure_asset(asset)
Expand Down
10 changes: 4 additions & 6 deletions src/iframe.jl
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
export iframe

mutable struct IFrame
innerHTML::AbstractString
bundleURL::AbstractString
innerHTML::String
bundleURL::String
end

iframe_bundle_key() = AssetRegistry.register(normpath(joinpath(
GENERIC_HTTP_BUNDLE_PATH
)))
iframe_bundle_key() = AssetRegistry.register(normpath(joinpath(GENERIC_HTTP_BUNDLE_PATH)))

"""
iframe(content)
Expand All @@ -30,7 +28,7 @@ function iframe(content)
dom = node(
:div,
node(ifr),
style=Dict(
style=Dict{Symbol,String}(
:display => "inherit",
:margin => "inherit",
),
Expand Down
2 changes: 1 addition & 1 deletion src/node.jl
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ struct Node{T}
end

function Node(instanceof, children...; props...)
return Node(instanceof, collect(Any, children), Dict(props...))
return Node(instanceof, collect(Any, children), Dict{Symbol,Any}(props...))
end

# Can/should this be deprecated?
Expand Down
4 changes: 2 additions & 2 deletions src/observable.jl
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
using JSON

struct ObservableNode
id::AbstractString
name::AbstractString
id::String
name::String
end
2 changes: 1 addition & 1 deletion src/rpc.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ registered_rpcs = Dict{UInt, Function}()
function tojs(f::Function)
h = hash(f)
registered_rpcs[h] = f
return js"WebIO.getRPC($(string(h)))"
return JSString("WebIO.getRPC($(string(h)))")
end

"""
Expand Down
28 changes: 14 additions & 14 deletions src/scope.jl
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import Sockets: send
import Observables: Observable, AbstractObservable, listeners

# bool marks if it is synced
const ObsDict = Dict{String, Tuple{AbstractObservable, Union{Nothing,Bool}}}
const ObsDict = Dict{String, Tuple{Observable, Union{Nothing,Bool}}}
Copy link
Member

Choose a reason for hiding this comment

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

Is this correct? Is it possible to register a non-concrete-Observable?

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure. It doesn't break anything and improves stability, but I'm not sure what the idea of having AbstractObservable is for, or if there are any of those in the wild.


mutable struct Scope
dom::Any
Expand All @@ -33,7 +33,7 @@ mutable struct Scope
# "observable-name" => ["array", "of", "JS", "strings"]
# where each JS-string is a function that is invoked when the observable
# changes.
jshandlers::Any
jshandlers::Dict{String,Any}
pool::ConnectionPool

mount_callbacks::Vector{JSString}
Expand All @@ -44,7 +44,7 @@ mutable struct Scope
private_obs::Set{String},
systemjs_options::Any,
imports::Vector{Asset},
jshandlers::Any,
jshandlers::Dict{String,Any},
pool::ConnectionPool,
mount_callbacks::Vector{JSString}
)
Expand Down Expand Up @@ -124,7 +124,7 @@ function Scope(;
private_obs::Set{String} = Set{String}(),
systemjs_options = nothing,
imports = Asset[],
jshandlers::Dict = Dict(),
jshandlers::Dict = Dict{String,Any}(),
mount_callbacks::Vector{JSString} = JSString[],
# Deprecated
id=nothing,
Expand Down Expand Up @@ -233,7 +233,7 @@ function Observable(ctx::Scope, key, val::T; sync=nothing) where {T}
end

function JSON.lower(scope::Scope)
obs_dict = Dict()
obs_dict = Dict{String,Dict{String,Any}}()
for (k, ob_) in scope.observs
ob, sync = ob_
if k in scope.private_obs
Expand All @@ -244,13 +244,13 @@ function JSON.lower(scope::Scope)
# other than the JS back edge
sync = any(f-> !isa(f, SyncCallback), listeners(ob))
end
obs_dict[k] = Dict(
obs_dict[k] = Dict{String,Any}(
"sync" => sync,
"value" => ob[],
"id" => obsid(ob)
)
end
Dict(
Dict{String,Any}(
"id" => scopeid(scope),
"systemjs_options" => scope.systemjs_options,
"imports" => lowerassets(scope.imports),
Expand Down Expand Up @@ -310,12 +310,12 @@ function onmount(scope::Scope, f::JSString)
end

function onimport(scope::Scope, f::JSString)
onmount(scope, js"""
onmount(scope, JSString("""
function () {
var handler = ($(f));
($(Async(scope.imports))).then((imports) => handler.apply(this, imports));
var handler = ($f);
($(tojs(Async(scope.imports)))).then((imports) => handler.apply(this, imports));
}
""")
"""))
end

onmount(scope::Scope, f) = onmount(scope, JSString(f))
Expand All @@ -326,9 +326,9 @@ Base.@deprecate ondependencies(ctx, jsf) onimport(ctx, jsf)
"""
A callable which updates the frontend
"""
struct SyncCallback
ctx
f
struct SyncCallback{C,F}
ctx::C
f::F
end

(s::SyncCallback)(xs...) = s.f(xs...)
Expand Down
4 changes: 2 additions & 2 deletions src/syntax.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ function cssparse(s)
# e.g. s = "img#theid.class1.class2[src=image.jpg, alt=great pic]"
# parse props first
p = match(r"\[[^\]]+\]", s)
props = Dict()
props = Dict{Symbol,Any}()
if p != nothing
m = strip(p.match, ['[',']'])
props[:attributes] = Dict(map(x->Pair(split(x,r"\s*=\s*", limit=2)...), split(m, r",\s*")))
Expand Down Expand Up @@ -207,7 +207,7 @@ const myStr = "foo"; const myObject = {"spam":"eggs","foo":"bar"};
"""
macro js_str(s)
writes = map(Interpolator(s)) do x
if isa(x, AbstractString)
if x isa AbstractString
# If x is a string, it was specified in the js"..." literal so let it
# through as-is.
:(write(io, $(x)))
Expand Down
12 changes: 6 additions & 6 deletions test/asset.jl
Original file line number Diff line number Diff line change
Expand Up @@ -46,33 +46,33 @@ using Test

@testset "Sync constructor" begin
single = Sync("foo.js")
@test typeof(single) === Sync
@test typeof(single) === Sync{Vector{Asset}}
@test single.imports[1] == Asset("foo.js")

double = Sync("foo" => "foo.js", Asset("bar.css"))
@test typeof(double) === Sync
@test typeof(double) === Sync{Vector{Asset}}
@test double.imports[1] == Asset("foo" => "foo.js")
@test double.imports[2] == Asset("bar.css")

nested = Sync(Sync("stepone.js", "steptwo.js"), "bar.css")
@test typeof(nested) === Sync
@test typeof(nested) === Sync{Vector{Any}}
@test nested.imports[1].imports[1] == Asset("stepone.js")
@test nested.imports[1].imports[2] == Asset("steptwo.js")
@test nested.imports[2] == Asset("bar.css")
end

@testset "Async constructor" begin
single = Async("foo.js")
@test typeof(single) === Async
@test typeof(single) === Async{Vector{Asset}}
@test single.imports[1] == Asset("foo.js")

double = Async("foo" => "foo.js", Asset("bar.css"))
@test typeof(double) === Async
@test typeof(double) === Async{Vector{Asset}}
@test double.imports[1] == Asset("foo" => "foo.js")
@test double.imports[2] == Asset("bar.css")

nested = Async(Sync("stepone.js", "steptwo.js"), "bar.css")
@test typeof(nested) === Async
@test typeof(nested) === Async{Vector{Any}}
@test nested.imports[1].imports[1] == Asset("stepone.js")
@test nested.imports[1].imports[2] == Asset("steptwo.js")
@test nested.imports[2] == Asset("bar.css")
Expand Down