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

Is the build step necessary? #46

Closed
4O4 opened this issue Oct 5, 2019 · 8 comments
Closed

Is the build step necessary? #46

4O4 opened this issue Oct 5, 2019 · 8 comments

Comments

@4O4
Copy link
Contributor

4O4 commented Oct 5, 2019

Hi, I've got to ask if the build step which concatenates everything to a single file is really necessary? I'm just finishing a quite big refactoring of this library in order to add proper automatic unsubscription (this also required to fix some of other reported issues) and now I'm realizing that the concatenation is gonna be pain in the ass.

When dependencies between classes become more complicated it's all fine until you use standard modular approach with require() but once you put everything into single file, the local ClassName declarations introduce problems with variables scoping. If one class is defined above the other one which it depends on, the script will simply fail. Reordering will not help with that, because the dependency tree is actually more complicated, a bit circular even and more classes are in play (kind of unavoidable).

So... should I worry about this? Can we simply skip building single-file rx.lua lib and replace it with standard init.lua file with all needed exports required into it? Or do you actually need it for some reason and if yes, then if you're willing to adjust the build script by yourself to handle the new layout. Personally I'd prefer the first option because it is cheap, easy and compatible with Lua packaging standards established by the community.

@bjornbytes
Copy link
Owner

Lua does not have "relative require" functionality, so it is harder for users to consume libraries that consist of multiple files.

If I place the rx folder at lib/rx in my app, all the files in the library need to be changed to require lib.rx.observer, etc. There are some tricks RxLua could use to work around this, like using ... in the main chunk to figure out what the module was required as and chopping off bits of the path to construct a relative path. However, to keep things simple and to make the library easier to use, it was decided to use an "amalgamation" script that concatenates the library into a single file.

If there are circular dependencies between the objects, I think these could be resolved by placing a line like this at the top of the single rx.lua file:

local Observer, Observable, Subscription -- etc.

This kinda "forward declares" the different objects so that they can all know about each other, regardless of which order they're defined in.

The concatenate script could be changed to add this line (and maybe remove the leading local that appears as the first word in each file, so that there aren't 2 definitions of local).

@4O4
Copy link
Contributor Author

4O4 commented Oct 5, 2019 via email

@4O4
Copy link
Contributor Author

4O4 commented Oct 5, 2019 via email

@bjornbytes
Copy link
Owner

One example of an environment that uses Lua but doesn't use luarocks/LUA_PATH is the LÖVE game engine and RxLove. In that situation games are distributed as standalone packages where the configuration of the target machine is unknown.

@4O4
Copy link
Contributor Author

4O4 commented Oct 6, 2019 via email

@bjornbytes
Copy link
Owner

I think it's a matter of preference. An artifact keeps the history clean, whereas committing the file makes it easier to use the library. My preference is to commit the file but I wouldn't really have strong objections to doing it a different way.

@4O4
Copy link
Contributor Author

4O4 commented Oct 7, 2019

Small update: I found that squish util (https://github.com/LuaDist/squish) could be handy and replace current build script. It produces single lua file with all specified modules rendered into it. I've made a test run and all modules are actually being preloaded to package.preload table so it should work natively with require() in most environments, because preload is the first thing being scanned during modules loading at runtime. Will that work in LOVE too?

@4O4
Copy link
Contributor Author

4O4 commented Oct 9, 2019

I've created #47 which addresses the issue with circular dependencies by using lua-amalg to create single-file lib as it seemed easier than maintaining the build script in the current form and adding handling of forward declarations to it.

@4O4 4O4 closed this as completed Oct 17, 2020
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

No branches or pull requests

2 participants