lapis
lapis copied to clipboard
Lua app.include is in a weird place
So I was rewatching the 2016 talk that Leaf did on Lapis and I really liked the sub-application stuff. I tried restructuring my application to use sub-applications and I ran into an error when using the below syntax:
error: ./app.lua:4: attempt to call method 'include' (a nil value)
I did a bit of investigating and found issue #255 that asks for the sub-application syntax to be added to Lua, not just Moonscript. This issue is from 2015 and the aforementioned talk was from 2016 so I figured it was added. I did a bit of digging through the Lapis lua code and I found this:
https://github.com/leafo/lapis/blob/master/lapis/application.lua#L287
It looks like include
is being added to __class
instead of __index
and thus is not able to be called normally. __init
, __base
, __name
, find_action
, and before_filter
are also located there, though before_filter
is duplicated in __index
too. the application itself only holds a singler property, router
.
When I try to call app.__class:include("apps.admin")
, I get the following error:
error: /usr/share/lua/5.1/lapis/application.lua:296: bad argument #1 to 'pairs' (table expected, got nil)
https://github.com/leafo/lapis/blob/master/lapis/application.lua#L296 shows that other_app
doesn't have a __base
property which is quite true, that is located in __class
as mentioned above.
So I'm not quite sure what to do here. I just spent a fair bit of time restructuring my code and I don't exactly want to undo all of that work. This is clearly a bug, but I'm not sure exactly what the bug is. Is Moonscript generating unexpectedly? From what I can tell with very minimal understanding of Moonscript, the function definitions prefixed with @
seem to be going into __class
whereas the rest are going into __index
. Is this intended behaviour? Is there a simple fix for this bug that could see a quick patch release of Lapis?
After a bit more tinkering, I got the following to work:
in the root application, I have the following code:
local lapis = require "lapis"
local app = lapis.Application()
app.include = app.__class.include
app:include("apps.core", nil, app)
return app
In the sub-applications, I have the following code:
local lapis = require "lapis"
local r2 = require("lapis.application").respond_to
local app = lapis.Application()
app.__base = app
app.name = "core."
app:match("home", "/", r2(require "apps.core.home"))
return app
Clearly not the best solution, but it seems to function (though I am scared to know what the internal data looks like and how this will bite me in the rear).
mmm, this was something I put into the talk thinking it would be ready by then, but I never had a chance to get it finalized. So it's not a bug, but a lost feature :(
The class-level include
method doesn't work on Lua based apps because of how they are instantiated:
- In MoonScript you create a subclass
- In Lua you instantiate the class, then add configuration data to the instance
This approach hasn't been ideal. Many of the class methods are copied over to be instance methods like you did in your fix above.
I want to make the Lua version work by creating subclasses, but it means changing the function you call to create an app, which would be a backwards incompatible change. After this change though, everything should work the same across both languages.
Any timeline on when those changes might be made?
Edit: FYI, I think a Lapis 2.0 release which brings MoonScript and Lua versions into complete feature parity would be a perfectly acceptable release, even if that's the only major change. Tagging it as 2.0 would also make the incompatible API more palitable. :)
I think that 2.0 should hopefully also include full 5.2 and 5.3 compatibility if possible.
I think this change makes sense for 2.0. Any other breaking changes on your wishlist?
Perhaps we should set up a v2.0 milestone that we can attach issues to so we can build a feature list/wishlist.
- Feature parity between Lua and MoonScript
- Lua 5.2 and 5.3 support
- Finish MySQL driver (I only bring this up because the issue tracking mysql support is 6/7 complete, not usre if this is out of date or just being deferred)
- Create SQLite driver
I think those are the major features I can think of, only the first (two?) being breaking changes.
Minor update on this. I noticed that sub-apps that have path prefixes require a path name, even if there is no name prefix set. Is this expected behaviour, or is this a quirk with my Lua hacks?
Closing outdated issue. The original issue was addressed in https://github.com/leafo/lapis/releases/tag/v1.11.0