red-blox / zap

A lightning fast networking solution for roblox.
https://zap.redblox.dev
MIT License
89 stars 14 forks source link

[FEAT] Safely handle requiring Zap in the Studio environment #92

Closed Cens-r closed 2 months ago

Cens-r commented 2 months ago

Describe your feature

Zap in its current state fails to account for the case where its modules (most significantly the client's) are required before runtime. When using plugins that may require modules that contain a require() to the client's Zap module you are currently met with an infinite yield.

This is because the client modules call WaitForChild on Zap's remotes:

local reliable = ReplicatedStorage:WaitForChild("ZAP_RELIABLE")
local unreliable = ReplicatedStorage:WaitForChild("ZAP_UNRELIABLE")

These remotes aren't created until Zap's server module is required for the first time.

I propose adding an environment check at the beginning of both modules to safely return some sort of mock table if they are required outside of a server or client context.

Alternatives

The alternative for this is creating the two events manually in ReplicatedStorage, either by requiring the server module or by constructing them yourself. As a temporary solution, in the current version (0.6) of Zap this is fairly simple and doable.

However, going forward with #72 for future versions (0.7+) this will introduce a problem as there will be an arbitrary number of events to keep up with. These events will increase and decrease in number between edits to the zap config.

Implementation Details

Zap should continue to return a table and maintain the typings regardless of whether it's loaded successfully or not. This load status of the module should be determined by whether the module is required in its correct environment (server or client) and not in the Studio environment.

This loaded state should be accessible outside of Zap module so that code that may be dependent on it can maneuver around any fatal errors that may arise from indexing for non-existent events.

An example implementation that should maintain typings:

-- Check context anything else
local loadedSuccessfully: boolean = --> Ensure context

if not loadedSuccessfully then
  local out = { Loaded = loadedSuccessfully }
  return (out :: any) :: ReturnType
end

-- Zap Module Code

-- Create return table:
local returns = {
  Loaded = loadedSuccessfully
  -- Event/Function Tables
}
type ReturnType = typeof(returns)

return returns
sasial-dev commented 2 months ago

I'll typecast it for 0.6.x, and then return noop functions in 0.7.0.

sasial-dev commented 2 months ago

I changed my mind, I'll go the noop route.

sasial-dev commented 2 months ago

The only thing is that due to the way zap is designed, it will return the noops in edit mode first before checking if it's on the server/client. But I think this is a suitable compromise before the rewrite.