Merge pull request #2845 from cloudflare/jsnell/tostringtag-compatflag

Add compat flag for the Symbol.toStringTag change

src/workerd/api/BUILD.bazel

@@ -547,3 +547,9 @@ wd_test(
     args = ["--experimental"],
     data = ["tests/error-in-error-event-test.js"],
 )
+
+wd_test(
+    src = "tests/no-to-string-tag-test.wd-test",
+    args = ["--experimental"],
+    data = ["tests/no-to-string-tag-test.js"],
+)

src/workerd/api/tests/global-scope-test.wd-test

@@ -8,7 +8,7 @@ const unitTests :Workerd.Config = (
           (name = "worker", esModule = embed "global-scope-test.js")
         ],
         compatibilityDate = "2023-01-15",
-        compatibilityFlags = ["nodejs_compat"]
+        compatibilityFlags = ["nodejs_compat", "set_tostring_tag"]
       )
     ),
   ],

src/workerd/api/tests/no-to-string-tag-test.js

@@ -0,0 +1,8 @@
+import { strictEqual } from 'node:assert';
+
+export default {
+  test() {
+    const h = new Headers();
+    strictEqual(h[Symbol.toStringTag], undefined);
+  },
+};

src/workerd/api/tests/no-to-string-tag-test.wd-test

@@ -0,0 +1,15 @@
+using Workerd = import "/workerd/workerd.capnp";
+
+const unitTests :Workerd.Config = (
+  services = [
+    ( name = "no-to-string-tag-test",
+      worker = (
+        modules = [
+          (name = "worker", esModule = embed "no-to-string-tag-test.js")
+        ],
+        compatibilityDate = "2023-01-15",
+        compatibilityFlags = ["nodejs_compat"]
+      )
+    ),
+  ],
+);

src/workerd/io/compatibility-date.capnp

@@ -622,4 +622,13 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef {
   # We plan to turn this on always quite soon. It would be an autogate but we need to test
   # our logic both at upload time and at runtime, and this seemed like the easiest way to
   # make sure we keep things in sync.
+
+  setToStringTag @64 :Bool
+      $compatEnableFlag("set_tostring_tag")
+      $compatDisableFlag("do_not_set_tostring_tag")
+      $compatEnableDate("2024-09-26");
+  # A change was made that set the Symbol.toStringTag on all jsg::Objects in order to
+  # fix several spec compliance bugs. Unfortunately it turns out that was more breaking
+  # than expected. This flag restores the original behavior for compat dates before
+  # 2024-09-26
 }

src/workerd/io/worker.c++

@@ -1028,6 +1028,9 @@ Worker::Isolate::Isolate(kj::Own<Api> apiParam,
 
     lock->setCaptureThrowsAsRejections(features.getCaptureThrowsAsRejections());
     lock->setCommonJsExportDefault(features.getExportCommonJsDefaultNamespace());
+    if (features.getSetToStringTag()) {
+      lock->setToStringTag();
+    }
     if (features.getNodeJsCompatV2()) {
       lock->setNodeJsCompatEnabled();
     }

src/workerd/jsg/jsg-test.c++

@@ -115,8 +115,6 @@ KJ_TEST("inheritance") {
   e.expectEval("newExtendedAsBase(123, 'foo') instanceof NumberBox", "boolean", "true");
 
   e.expectEval("newExtendedAsBase(123, 'foo') instanceof ExtendedNumberBox", "boolean", "true");
-
-  e.expectEval("(new Other())[Symbol.toStringTag]", "string", "Other");
 }
 
 // ========================================================================================

src/workerd/jsg/jsg.c++

@@ -195,6 +195,10 @@ void Lock::setNodeJsCompatEnabled() {
   IsolateBase::from(v8Isolate).setNodeJsCompatEnabled({}, true);
 }
 
+void Lock::setToStringTag() {
+  IsolateBase::from(v8Isolate).enableSetToStringTag();
+}
+
 void Lock::setCommonJsExportDefault(bool exportDefault) {
   IsolateBase::from(v8Isolate).setCommonJsExportDefault({}, exportDefault);
 }

src/workerd/jsg/jsg.h

@@ -2514,6 +2514,7 @@ class Lock {
   void setCommonJsExportDefault(bool exportDefault);
 
   void setNodeJsCompatEnabled();
+  void setToStringTag();
 
   using Logger = void(Lock&, kj::StringPtr);
   void setLoggerCallback(kj::Function<Logger>&& logger);

src/workerd/jsg/resource.h

@@ -1533,7 +1533,10 @@ class ResourceWrapper {
 
     auto classname = v8StrIntern(isolate, typeName(typeid(T)));
 
-    prototype->Set(v8::Symbol::GetToStringTag(isolate), classname, v8::PropertyAttribute::DontEnum);
+    if (getShouldSetToStringTag(isolate)) {
+      prototype->Set(
+          v8::Symbol::GetToStringTag(isolate), classname, v8::PropertyAttribute::DontEnum);
+    }
 
     // Previously, miniflare would use the lack of a Symbol.toStringTag on a class to
     // detect a type that came from the runtime. That's obviously a bit problematic because

src/workerd/jsg/setup.h

@@ -143,6 +143,14 @@ class IsolateBase {
     return nodeJsCompatEnabled;
   }
 
+  inline bool shouldSetToStringTag() const {
+    return setToStringTag;
+  }
+
+  void enableSetToStringTag() {
+    setToStringTag = true;
+  }
+
   // The logger will be optionally set by the isolate setup logic if there is anywhere
   // for the log to go (for instance, if debug logging is enabled or the inspector is
   // being used).
@@ -229,6 +237,7 @@ class IsolateBase {
   bool exportCommonJsDefault = false;
   bool asyncContextTrackingEnabled = false;
   bool nodeJsCompatEnabled = false;
+  bool setToStringTag = false;
 
   kj::Maybe<kj::Function<Logger>> maybeLogger;
   kj::Maybe<kj::Function<ErrorReporter>> maybeErrorReporter;

src/workerd/jsg/util.c++

@@ -29,6 +29,11 @@ bool getCommonJsExportDefault(v8::Isolate* isolate) {
   return jsgIsolate.getCommonJsExportDefault();
 }
 
+bool getShouldSetToStringTag(v8::Isolate* isolate) {
+  auto& jsgIsolate = *reinterpret_cast<IsolateBase*>(isolate->GetData(0));
+  return jsgIsolate.shouldSetToStringTag();
+}
+
 #if _WIN32
 kj::String fullyQualifiedTypeName(const std::type_info& type) {
   // type.name() returns a human-readable name on Windows:

src/workerd/jsg/util.h

@@ -45,6 +45,7 @@ class JsExceptionThrown: public std::exception {
 
 bool getCaptureThrowsAsRejections(v8::Isolate* isolate);
 bool getCommonJsExportDefault(v8::Isolate* isolate);
+bool getShouldSetToStringTag(v8::Isolate* isolate);
 
 kj::String fullyQualifiedTypeName(const std::type_info& type);
 kj::String typeName(const std::type_info& type);