-
Notifications
You must be signed in to change notification settings - Fork 143
validateProps #94
validateProps #94
Changes from 25 commits
47350e4
61614d4
567d583
4f6d3ab
136890b
e1332c3
5f93de2
bb62d66
226c546
15c8a78
4f811d7
5802c1f
a59233d
6c8ae89
8e411d9
8f62b6e
2c50417
74990f4
255bc80
4643508
f9d2594
e7afda4
36df87c
ca9cc17
417d568
ca6e364
a600bac
36cad55
8933cf0
8f7d97b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,6 +77,7 @@ function Component:extend(name) | |
end | ||
|
||
class.__index = class | ||
class._extendTraceback = debug.traceback() | ||
|
||
setmetatable(class, { | ||
__tostring = function(self) | ||
|
@@ -309,6 +310,10 @@ function Component:_forceUpdate(newProps, newState) | |
end | ||
end | ||
|
||
if newProps then | ||
self:_validateProps(newProps) | ||
end | ||
|
||
if self.willUpdate then | ||
self._setStateBlockedReason = "willUpdate" | ||
self:willUpdate(newProps or self.props, newState or self.state) | ||
|
@@ -374,6 +379,8 @@ function Component:_mount(handle) | |
|
||
self._setStateBlockedReason = "render" | ||
|
||
self:_validateProps(self.props) | ||
|
||
local virtualTree | ||
if GlobalConfig.getValue("componentInstrumentation") then | ||
local startTime = tick() | ||
|
@@ -424,4 +431,35 @@ function Component:_unmount() | |
self._handle = nil | ||
end | ||
|
||
--[[ | ||
Performs property validation, if it is enabled. | ||
]] | ||
function Component:_validateProps(props) | ||
if not GlobalConfig.getValue("propValidation") then | ||
return | ||
end | ||
|
||
local validator = self.validateProps | ||
|
||
if validator == nil then return end | ||
if typeof(validator) ~= "function" then | ||
-- Hide as much as possible about error location, since this message | ||
-- occurs from several possible call sites with different stack traces. | ||
error(("The value of validateProps must be a function, but it is a %s.\n".. | ||
"Check the definition of the component extended at:\n%s"):format( | ||
typeof(validator), | ||
self._extendTraceback), | ||
0) | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this totally necessary? We don't bother doing this with lifecycle methods or things like I can see the argument for doing it here since you're already opting into validation, just wondering what the thought process is since it's not something we do elsewhere. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most of the time when function SomeComponent.validateProps(props)
-- do some validation work
end Instead, it's usually composed from a bunch of smaller validators, and the result of the composition is assigned. When you're using PropTypes it looks something like this: SomeComponent.validateProps = PropTypes.object {
someKey = PropTypes.number,
someOtherKey = PropTypes.string,
} The type check is an extra guard against There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gotcha, that makes sense! |
||
|
||
local success, failureReason = validator(props) | ||
|
||
if not success then | ||
failureReason = failureReason or "<No failure reason was given by the validation function>" | ||
error(("Property validation failed:\n%s\n%s"):format( | ||
tostring(failureReason), | ||
self:getElementTraceback() or Core._defaultElementTracebackMessage), 0) | ||
end | ||
end | ||
|
||
return Component |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put
return
on its own line? We try to follow one statement per line!