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

Repo refactors for Fusion #40

Open
dphfox opened this issue Sep 3, 2021 · 7 comments
Open

Repo refactors for Fusion #40

dphfox opened this issue Sep 3, 2021 · 7 comments
Assignees
Labels
enhancement New feature or request ready to work on Enhancements/changes ready to be made

Comments

@dphfox
Copy link
Owner

dphfox commented Sep 3, 2021

I've heard some people here express concerns about namespace pollution, particularly when talking about Fusion providing utilities that some projects may want to keep a custom implementation of.

One of my visions for how Fusion would work is that it'd be largely complete out of the box, providing everything you need for building most kinds of UI. This would remove the need to search for, choose and download a bunch of libraries just to get started, and help make different Fusion projects more consistent and understandable by sharing a lot of widely used and understood APIs.

I acknowledge however this may have some side effects that could be undesirable:

  • More modules included in Fusion by default means larger file sizes
  • These utilities sharing a namespace could introduce name collision problems (though this can be avoided at the API design stage)
  • Extra unneeded code for projects that replace some utilities

In addition, the extension story for Fusion is rather unclear:

  • How does a third party library know where Fusion is located?
  • How do these libraries extend Fusion when many core APIs for reactive updates and dependency management are not exposed?
  • What standard interfaces should there be between libraries, for example for state objects?

Given these points, it might be worth moving Fusion to a more modular design. Here's a rough idea of how that could work:

  • The official Fusion codebase is split into multiple modules, which can be freely added and removed by end users:
    • Core for fundamental state management (e.g. reactive graph updating, dependency management, cleanup utils, State + Computed)
    • State for extended state tools and utilities (e.g. ComputedPairs, makeState)
    • Instances for Roblox instance APIs (e.g. New, Scheduler, Hydrate)
    • Motion for animation and motion design tools (e.g. Tween, Spring, Keyframes, Timer, Oklab)
  • These modules are contained in ModuleScripts which are parented directly under the main Fusion module. This means that these modules always know where your Fusion module is (and also enables possibly using multiple Fusion installations side by side, which would be particularly useful in dev environments)
  • The default Fusion package includes all modules by default, so people starting out or prototyping with Fusion get the same DX as they do today. However, interested users can remove modules they don't use or replace modules with their own implementation.
  • Other developers making their own third party libraries can leverage this module system to make libraries that can be easily 'plugged into' any Fusion installation.
  • These modules would be required and exposed through the main Fusion module, though the exact process by which this should happen is unclear. Ideally, we should optimise for these use cases at least:
    • Some people use Fusion 'fully qualified'; we don't recommend this because it's especially sensitive to changes like this. e.g. local ui = Fusion.New(...)
    • Code right now expects to find Fusion APIs directly under the main Fusion module
    • Different modules may have name collisions - this is something that should be intentionally avoided for official modules anyway, but third party libraries complicate this

Is this worth pursuing?

@dphfox dphfox added enhancement New feature or request not ready - evaluating Currently gauging feedback labels Sep 3, 2021
@Anaminus
Copy link
Contributor

Anaminus commented Sep 3, 2021

I see two approaches:

Inward-out

Fusion is required by plugins.

  • Plugins themselves require the Fusion plugin API.
  • By having a common location, plugins can assume the location of the plugin API, as well as other plugins.
  • Plugins must be required directly by users.

This seems rather clunky, unsafe, and doesn't really amount to anything more than deciding on a known location for plugins.

Outward-in

Plugins are required by Fusion.

  • Fusion exposes an API for users to require lazily loaded plugins (bonus: free safe descension).
  • The plugin API also includes a means for plugins to require other plugins.
    • In order to avoid circular dependencies, this must be separate from the require API for users. That is, a plugin cannot be a user. In general, the plugin API must be distinct from the user API.
    • Alternatively, a plugin would have be structured to return a function that receives the plugin API as an argument, without touching Fusion while initializing.

By having Fusion faciliate plugin loading, some interesting things can be done.

  • Plugins could be configured to determine how they can be used (via attribute?).
    • Plugin: whether the plugin can be loaded and accessed by other plugins.
    • User: whether the plugin can be loaded and accessed by users.
  • Another setting might be to force a plugin to load on startup rather than lazily.

@filiptibell
Copy link

filiptibell commented Sep 4, 2021

I would love a more extensible Fusion, where more functionality can be added on top as needed.

Here's a rough idea of how that could work...

That rough idea seems like a reasonable approach to me. What I found to be lacking the most when making utilities and helpers for writing Fusion code has definitely been a public API for core functionality.
My main gripes have been figuring out "is this passed arg to my utility function a State/Computed/etc?", as well as knowing when to perform cleanup tasks related to instances.

  • These modules would be required and exposed through the main Fusion module, though the exact process by which this should happen is unclear. Ideally, we should optimise for these use cases at least:
    • Some people use Fusion 'fully qualified'; we don't recommend this because it's especially sensitive to changes like this. e.g. local ui = Fusion.New(...)
    • Code right now expects to find Fusion APIs directly under the main Fusion module

Making changes that affect this still seems fine to do since the only release so far has been an experimental beta; everyone should expect things to change, even at the most basic level.
All of the Fusion docs and tutorials already prompt us to have top-level imports of Fusion APIs, so while changing the import paths would be a breaking change, as long as it is clearly mentioned in release notes, IMO it is completely fine. An imports migration guide would be nice but not necessary.

Plugins are required by Fusion.

  • Fusion exposes an API for users to require lazily loaded plugins (bonus: free safe descension).

I personally would not be a fan of lazy loading and loose coupling in the context of this library. Especially in Roblox, it tends to give a far worse developer experience, with intellisense completely disappearing + there would be an added need to learn how a custom loader works and what quirks it would have. Arguably, intellisense and similar issues are things that could improve later on, but that would be dependent on Roblox/ a custom LSP and making it extensible enough 😕

Lazy loading definitely has its place, and can be great for a generic library loader / require() replacement if that is something you like, but having it within a library that is focused on solving other kinds of problems is not something I agree with.

@dphfox
Copy link
Owner Author

dphfox commented Sep 4, 2021

I personally would not be a fan of lazy loading and loose coupling in the context of this library. Especially in Roblox, it tends to give a far worse developer experience, with intellisense completely disappearing + there would be an added need to learn how a custom loader works and what quirks it would have. Arguably, intellisense and similar issues are things that could improve later on, but that would be dependent on Roblox/ a custom LSP and making it extensible enough 😕

Lazy loading definitely has its place, and can be great for a generic library loader / require() replacement if that is something you like, but having it within a library that is focused on solving other kinds of problems is not something I agree with.

This is a good point for consideration - we definitely don't want to break intellisense. Thanks for bringing it up!

@dphfox
Copy link
Owner Author

dphfox commented Mar 1, 2022

Deferring to v0.3

@dphfox
Copy link
Owner Author

dphfox commented Aug 22, 2023

So I've been ruminating on this ever since I kicked this particular can down the v0.3 road. I think the right move is probably to keep it simple. Instead of defining a fancy plugin system or loader system, it's perhaps just best to specify that Fusion modules are merely parented under the same instance or directory. This would probably mean people will have to require multiple different libraries to access e.g. Roblox specific extensions, but also it's probably not worth pursuing anything clever.

I'd also like to further clarify what exactly is the boundary of the Fusion project because it's never really been well defined. What is the scope of Fusion? Perhaps Fusion should encompass every common use case for the reactive graph tech at its core, regardless of whether it's used for UI, or backend game logic, or motion design....

This'll probably happen once v0.3 is feature complete so it doesn't cause a bunch of extra work in other branches and PRs. I'm going to pull in some other suggestions from elsewhere, and take the opportunity to roll in the switch to non-Roblox Luau + Darklua for Roblox transcription, as well as the luau file extension change that was also proposed. Easier to do it all at once this way.

I hope that this could also lead to a better CI/testing situation since we won't have to rely on hacks to get Roblox to cooperate with us and run our code in Actions.

@dphfox dphfox changed the title Modular Fusion Repo refactors for v0.3 Aug 22, 2023
@dphfox dphfox added not ready - blocked Waiting on other work to be done and removed not ready - evaluating Currently gauging feedback labels Aug 22, 2023
@dphfox dphfox pinned this issue Aug 29, 2023
@dphfox
Copy link
Owner Author

dphfox commented Aug 29, 2023

I'll take the opportunity to update the style guide too, since it's been decaying for years. Here's a draft of what I'm working on:

Fusion Style Guide.md

Hopefully it's more concise and easier to understand.

@dphfox dphfox unpinned this issue Apr 14, 2024
@dphfox
Copy link
Owner Author

dphfox commented Sep 1, 2024

We're ready to go ahead with this for next update. There's some other motivating forces at play now - there's more desire for external libraries to augment and replace parts of Fusion now, so we'd do well to accommodate them.

We've also done well to cut down on fully qualified use of Fusion thanks to how scopes allow method imports in more central locations. Most code files need not import most of Fusion.

@dphfox dphfox removed the not ready - blocked Waiting on other work to be done label Sep 1, 2024
@dphfox dphfox added the ready to work on Enhancements/changes ready to be made label Sep 1, 2024
@dphfox dphfox changed the title Repo refactors for v0.3 Repo refactors for Fusion Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready to work on Enhancements/changes ready to be made
Projects
None yet
Development

No branches or pull requests

4 participants
@Anaminus @filiptibell @dphfox and others