Skip to content

Conversation

@pyramation
Copy link
Collaborator

No description provided.

@pyramation pyramation changed the base branch from 16-latest to 17-latest-new-base-from-15 June 3, 2025 18:12
@pyramation
Copy link
Collaborator Author

Cannot set properties of undefined in proto.js

Summary

When deparsing an INSERT statement with multiple VALUES lists, the generated Protobuf helper code in proto.js threw:

TypeError: Cannot set properties of undefined (setting '0')
  at String (proto.js:10726:43)

This turned out to be a naming collision between the Protobuf message type String (and BitString) and the JavaScript built-in String constructor.


How the Bug Occurred

  1. Protobuf definitions include a message:
    message String {
      string sval = 1;
    }
  2. pbjs code-generation creates a constructor function named String(properties) inside proto.js (shadowing the global String). It also emits a static fromObject(object) method that does:
    String.fromObject = function(object) {
      var message = new $root.pg_query.String();
      if (object.sval != null)
        message.sval = String(object.sval);
      return message;
    };
  3. The unqualified call to String(object.sval) inside fromObject resolves to the local Protobuf wrapper constructor, not the global primitive conversion. That invocation happens as
    String('John');
    i.e. without new, so inside the wrapper constructor this is undefined. When properties is non‐null (a string is truthy), it tries:
    for (var keys = Object.keys(properties), ...)
      this[keys[i]] = properties[keys[i]];
    and this being undefined triggers the TypeError.

Why Our Patch Works

We replaced the problematic lines in both String.fromObject and BitString.fromObject:

-  if (object.sval != null)
-    message.sval = String(object.sval);
+  if (object.sval != null)
+    message.sval = object.sval;

This change:

  • Avoids calling the shadowed String constructor entirely. We simply take the primitive string out of the JSON and hand it straight into the Protobuf message.
  • Prevents accidental function calls without new (so no bad this binding).

Why This Might Be Brittle

  • Regeneration risk: Running node script/protogen.js will re-write proto.js from scratch, undoing this manual patch. You'll need a post‐generation step or a more permanent code-generator fix.
  • Naming collision: Any Protobuf message named String, Boolean, Array, etc., can shadow JS globals. A generator option to rename these or to explicitly call global constructors (globalThis.String(...)) would be more robust.

Recommendations

  1. Permanent Generator Fix: Update the pbjs or our wrapper generator to emit globalThis.String(object.sval) or String.prototype.toString.call(object.sval) (i.e. reference the global), or rename the PB type (e.g. PgString).
  2. Post‐process Script: Add a sed or AST‐based transform to re-patch proto.js every time you regen.
  3. Avoid Shadowed Names: In your .proto, avoid reserved names that collide with built-ins, or choose a different prefix for primitive wrappers.

@pyramation
Copy link
Collaborator Author

pyramation commented Jun 3, 2025

  • update @pgsql/types

@pyramation pyramation marked this pull request as ready for review June 3, 2025 19:12
@pyramation pyramation merged commit 37b40ed into 17-latest-new-base-from-15 Jun 3, 2025
@pyramation pyramation deleted the 17-upgrade-from-15-latest branch June 22, 2025 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants