Skip to content

Commit 03b17d9

Browse files
neildharfacebook-github-bot
authored andcommitted
Clarify const-ness of JSI references
Summary: Many operations on references in JS can modify the referent by executing additional JS, including operations that we currently mark as const such as `getProperty`. Because of this, the current distinction between const and non-const operations on references like `jsi::Object` is somewhat arbitrary. A more consistent approach is to mark all operations as const, so that it is clear that the const-ness applies to the reference and not the referent. This is analogous to how smart pointers work, since something like `const shared_ptr<T>` only makes the pointer const, as opposed to `shared_ptr<const T>`. This also gives users better guarantees and more flexibility in where these references may be used. Changelog: [General][Changed] - Mark methods on JSI references as const. Reviewed By: fbmal7 Differential Revision: D41599116 fbshipit-source-id: 40b1537581b09c5a34d0928ee04e7db2b50d26ea
1 parent c4862a2 commit 03b17d9

File tree

4 files changed

+56
-40
lines changed

4 files changed

+56
-40
lines changed

ReactCommon/jsc/JSCRuntime.cpp

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -187,11 +187,11 @@ class JSCRuntime : public jsi::Runtime {
187187
bool hasProperty(const jsi::Object &, const jsi::String &name) override;
188188
bool hasProperty(const jsi::Object &, const jsi::PropNameID &name) override;
189189
void setPropertyValue(
190-
jsi::Object &,
190+
const jsi::Object &,
191191
const jsi::String &name,
192192
const jsi::Value &value) override;
193193
void setPropertyValue(
194-
jsi::Object &,
194+
const jsi::Object &,
195195
const jsi::PropNameID &name,
196196
const jsi::Value &value) override;
197197
bool isArray(const jsi::Object &) const override;
@@ -203,7 +203,7 @@ class JSCRuntime : public jsi::Runtime {
203203

204204
// TODO: revisit this implementation
205205
jsi::WeakObject createWeakObject(const jsi::Object &) override;
206-
jsi::Value lockWeakObject(jsi::WeakObject &) override;
206+
jsi::Value lockWeakObject(const jsi::WeakObject &) override;
207207

208208
jsi::Array createArray(size_t length) override;
209209
jsi::ArrayBuffer createArrayBuffer(
@@ -212,8 +212,10 @@ class JSCRuntime : public jsi::Runtime {
212212
size_t size(const jsi::ArrayBuffer &) override;
213213
uint8_t *data(const jsi::ArrayBuffer &) override;
214214
jsi::Value getValueAtIndex(const jsi::Array &, size_t i) override;
215-
void setValueAtIndexImpl(jsi::Array &, size_t i, const jsi::Value &value)
216-
override;
215+
void setValueAtIndexImpl(
216+
const jsi::Array &,
217+
size_t i,
218+
const jsi::Value &value) override;
217219

218220
jsi::Function createFunctionFromHostFunction(
219221
const jsi::PropNameID &name,
@@ -949,7 +951,7 @@ bool JSCRuntime::hasProperty(
949951
}
950952

951953
void JSCRuntime::setPropertyValue(
952-
jsi::Object &object,
954+
const jsi::Object &object,
953955
const jsi::PropNameID &name,
954956
const jsi::Value &value) {
955957
JSValueRef exc = nullptr;
@@ -964,7 +966,7 @@ void JSCRuntime::setPropertyValue(
964966
}
965967

966968
void JSCRuntime::setPropertyValue(
967-
jsi::Object &object,
969+
const jsi::Object &object,
968970
const jsi::String &name,
969971
const jsi::Value &value) {
970972
JSValueRef exc = nullptr;
@@ -1064,7 +1066,7 @@ jsi::WeakObject JSCRuntime::createWeakObject(const jsi::Object &obj) {
10641066
#endif
10651067
}
10661068

1067-
jsi::Value JSCRuntime::lockWeakObject(jsi::WeakObject &obj) {
1069+
jsi::Value JSCRuntime::lockWeakObject(const jsi::WeakObject &obj) {
10681070
#ifdef RN_FABRIC_ENABLED
10691071
// TODO: revisit this implementation
10701072
JSObjectRef objRef = objectRef(obj);
@@ -1107,7 +1109,7 @@ jsi::Value JSCRuntime::getValueAtIndex(const jsi::Array &arr, size_t i) {
11071109
}
11081110

11091111
void JSCRuntime::setValueAtIndexImpl(
1110-
jsi::Array &arr,
1112+
const jsi::Array &arr,
11111113
size_t i,
11121114
const jsi::Value &value) {
11131115
JSValueRef exc = nullptr;

ReactCommon/jsi/jsi/decorator.h

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -264,11 +264,13 @@ class RuntimeDecorator : public Base, private jsi::Instrumentation {
264264
bool hasProperty(const Object& o, const String& name) override {
265265
return plain_.hasProperty(o, name);
266266
};
267-
void setPropertyValue(Object& o, const PropNameID& name, const Value& value)
268-
override {
267+
void setPropertyValue(
268+
const Object& o,
269+
const PropNameID& name,
270+
const Value& value) override {
269271
plain_.setPropertyValue(o, name, value);
270272
};
271-
void setPropertyValue(Object& o, const String& name, const Value& value)
273+
void setPropertyValue(const Object& o, const String& name, const Value& value)
272274
override {
273275
plain_.setPropertyValue(o, name, value);
274276
};
@@ -295,7 +297,7 @@ class RuntimeDecorator : public Base, private jsi::Instrumentation {
295297
WeakObject createWeakObject(const Object& o) override {
296298
return plain_.createWeakObject(o);
297299
};
298-
Value lockWeakObject(WeakObject& wo) override {
300+
Value lockWeakObject(const WeakObject& wo) override {
299301
return plain_.lockWeakObject(wo);
300302
};
301303

@@ -318,7 +320,8 @@ class RuntimeDecorator : public Base, private jsi::Instrumentation {
318320
Value getValueAtIndex(const Array& a, size_t i) override {
319321
return plain_.getValueAtIndex(a, i);
320322
};
321-
void setValueAtIndexImpl(Array& a, size_t i, const Value& value) override {
323+
void setValueAtIndexImpl(const Array& a, size_t i, const Value& value)
324+
override {
322325
plain_.setValueAtIndexImpl(a, i, value);
323326
};
324327

@@ -656,12 +659,14 @@ class WithRuntimeDecorator : public RuntimeDecorator<Plain, Base> {
656659
Around around{with_};
657660
return RD::hasProperty(o, name);
658661
};
659-
void setPropertyValue(Object& o, const PropNameID& name, const Value& value)
660-
override {
662+
void setPropertyValue(
663+
const Object& o,
664+
const PropNameID& name,
665+
const Value& value) override {
661666
Around around{with_};
662667
RD::setPropertyValue(o, name, value);
663668
};
664-
void setPropertyValue(Object& o, const String& name, const Value& value)
669+
void setPropertyValue(const Object& o, const String& name, const Value& value)
665670
override {
666671
Around around{with_};
667672
RD::setPropertyValue(o, name, value);
@@ -696,7 +701,7 @@ class WithRuntimeDecorator : public RuntimeDecorator<Plain, Base> {
696701
Around around{with_};
697702
return RD::createWeakObject(o);
698703
};
699-
Value lockWeakObject(WeakObject& wo) override {
704+
Value lockWeakObject(const WeakObject& wo) override {
700705
Around around{with_};
701706
return RD::lockWeakObject(wo);
702707
};
@@ -725,7 +730,8 @@ class WithRuntimeDecorator : public RuntimeDecorator<Plain, Base> {
725730
Around around{with_};
726731
return RD::getValueAtIndex(a, i);
727732
};
728-
void setValueAtIndexImpl(Array& a, size_t i, const Value& value) override {
733+
void setValueAtIndexImpl(const Array& a, size_t i, const Value& value)
734+
override {
729735
Around around{with_};
730736
RD::setValueAtIndexImpl(a, i, value);
731737
};

ReactCommon/jsi/jsi/jsi-inl.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,19 +111,21 @@ inline bool Object::hasProperty(Runtime& runtime, const PropNameID& name)
111111
}
112112

113113
template <typename T>
114-
void Object::setProperty(Runtime& runtime, const char* name, T&& value) {
114+
void Object::setProperty(Runtime& runtime, const char* name, T&& value) const {
115115
setProperty(
116116
runtime, String::createFromAscii(runtime, name), std::forward<T>(value));
117117
}
118118

119119
template <typename T>
120-
void Object::setProperty(Runtime& runtime, const String& name, T&& value) {
120+
void Object::setProperty(Runtime& runtime, const String& name, T&& value)
121+
const {
121122
setPropertyValue(
122123
runtime, name, detail::toValue(runtime, std::forward<T>(value)));
123124
}
124125

125126
template <typename T>
126-
void Object::setProperty(Runtime& runtime, const PropNameID& name, T&& value) {
127+
void Object::setProperty(Runtime& runtime, const PropNameID& name, T&& value)
128+
const {
127129
setPropertyValue(
128130
runtime, name, detail::toValue(runtime, std::forward<T>(value)));
129131
}
@@ -229,12 +231,12 @@ inline Array Object::getPropertyNames(Runtime& runtime) const {
229231
return runtime.getPropertyNames(*this);
230232
}
231233

232-
inline Value WeakObject::lock(Runtime& runtime) {
234+
inline Value WeakObject::lock(Runtime& runtime) const {
233235
return runtime.lockWeakObject(*this);
234236
}
235237

236238
template <typename T>
237-
void Array::setValueAtIndex(Runtime& runtime, size_t i, T&& value) {
239+
void Array::setValueAtIndex(Runtime& runtime, size_t i, T&& value) const {
238240
setValueAtIndexImpl(
239241
runtime, i, detail::toValue(runtime, std::forward<T>(value)));
240242
}

ReactCommon/jsi/jsi/jsi.h

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -336,10 +336,12 @@ class JSI_EXPORT Runtime {
336336
virtual Value getProperty(const Object&, const String& name) = 0;
337337
virtual bool hasProperty(const Object&, const PropNameID& name) = 0;
338338
virtual bool hasProperty(const Object&, const String& name) = 0;
339+
virtual void setPropertyValue(
340+
const Object&,
341+
const PropNameID& name,
342+
const Value& value) = 0;
339343
virtual void
340-
setPropertyValue(Object&, const PropNameID& name, const Value& value) = 0;
341-
virtual void
342-
setPropertyValue(Object&, const String& name, const Value& value) = 0;
344+
setPropertyValue(const Object&, const String& name, const Value& value) = 0;
343345

344346
virtual bool isArray(const Object&) const = 0;
345347
virtual bool isArrayBuffer(const Object&) const = 0;
@@ -349,7 +351,7 @@ class JSI_EXPORT Runtime {
349351
virtual Array getPropertyNames(const Object&) = 0;
350352

351353
virtual WeakObject createWeakObject(const Object&) = 0;
352-
virtual Value lockWeakObject(WeakObject&) = 0;
354+
virtual Value lockWeakObject(const WeakObject&) = 0;
353355

354356
virtual Array createArray(size_t length) = 0;
355357
virtual ArrayBuffer createArrayBuffer(
@@ -358,7 +360,8 @@ class JSI_EXPORT Runtime {
358360
virtual size_t size(const ArrayBuffer&) = 0;
359361
virtual uint8_t* data(const ArrayBuffer&) = 0;
360362
virtual Value getValueAtIndex(const Array&, size_t i) = 0;
361-
virtual void setValueAtIndexImpl(Array&, size_t i, const Value& value) = 0;
363+
virtual void
364+
setValueAtIndexImpl(const Array&, size_t i, const Value& value) = 0;
362365

363366
virtual Function createFunctionFromHostFunction(
364367
const PropNameID& name,
@@ -666,7 +669,7 @@ class JSI_EXPORT Object : public Pointer {
666669
}
667670

668671
/// \return the result of `this instanceOf ctor` in JS.
669-
bool instanceOf(Runtime& rt, const Function& ctor) {
672+
bool instanceOf(Runtime& rt, const Function& ctor) const {
670673
return rt.instanceOf(*this, ctor);
671674
}
672675

@@ -701,19 +704,19 @@ class JSI_EXPORT Object : public Pointer {
701704
/// used to make one: nullptr_t, bool, double, int, const char*,
702705
/// String, or Object.
703706
template <typename T>
704-
void setProperty(Runtime& runtime, const char* name, T&& value);
707+
void setProperty(Runtime& runtime, const char* name, T&& value) const;
705708

706709
/// Sets the property value from a Value or anything which can be
707710
/// used to make one: nullptr_t, bool, double, int, const char*,
708711
/// String, or Object.
709712
template <typename T>
710-
void setProperty(Runtime& runtime, const String& name, T&& value);
713+
void setProperty(Runtime& runtime, const String& name, T&& value) const;
711714

712715
/// Sets the property value from a Value or anything which can be
713716
/// used to make one: nullptr_t, bool, double, int, const char*,
714717
/// String, or Object.
715718
template <typename T>
716-
void setProperty(Runtime& runtime, const PropNameID& name, T&& value);
719+
void setProperty(Runtime& runtime, const PropNameID& name, T&& value) const;
717720

718721
/// \return true iff JS \c Array.isArray() would return \c true. If
719722
/// so, then \c getArray() will succeed.
@@ -832,15 +835,17 @@ class JSI_EXPORT Object : public Pointer {
832835
Array getPropertyNames(Runtime& runtime) const;
833836

834837
protected:
835-
void
836-
setPropertyValue(Runtime& runtime, const String& name, const Value& value) {
838+
void setPropertyValue(
839+
Runtime& runtime,
840+
const String& name,
841+
const Value& value) const {
837842
return runtime.setPropertyValue(*this, name, value);
838843
}
839844

840845
void setPropertyValue(
841846
Runtime& runtime,
842847
const PropNameID& name,
843-
const Value& value) {
848+
const Value& value) const {
844849
return runtime.setPropertyValue(*this, name, value);
845850
}
846851

@@ -866,7 +871,7 @@ class JSI_EXPORT WeakObject : public Pointer {
866871
/// otherwise returns \c undefined. Note that this method has nothing to do
867872
/// with threads or concurrency. The name is based on std::weak_ptr::lock()
868873
/// which serves a similar purpose.
869-
Value lock(Runtime& runtime);
874+
Value lock(Runtime& runtime) const;
870875

871876
friend class Runtime;
872877
};
@@ -902,7 +907,7 @@ class JSI_EXPORT Array : public Object {
902907
/// value behaves as with Object::setProperty(). If \c i is out of
903908
/// range [ 0..\c length ] throws a JSIException.
904909
template <typename T>
905-
void setValueAtIndex(Runtime& runtime, size_t i, T&& value);
910+
void setValueAtIndex(Runtime& runtime, size_t i, T&& value) const;
906911

907912
/// There is no current API for changing the size of an array once
908913
/// created. We'll probably need that eventually.
@@ -920,7 +925,8 @@ class JSI_EXPORT Array : public Object {
920925
friend class Object;
921926
friend class Value;
922927

923-
void setValueAtIndexImpl(Runtime& runtime, size_t i, const Value& value) {
928+
void setValueAtIndexImpl(Runtime& runtime, size_t i, const Value& value)
929+
const {
924930
return runtime.setValueAtIndexImpl(*this, i, value);
925931
}
926932

@@ -946,7 +952,7 @@ class JSI_EXPORT ArrayBuffer : public Object {
946952
return runtime.size(*this);
947953
}
948954

949-
uint8_t* data(Runtime& runtime) {
955+
uint8_t* data(Runtime& runtime) const {
950956
return runtime.data(*this);
951957
}
952958

0 commit comments

Comments
 (0)