Upgrade js-yaml from 4.x to 5.x in packages/mql-typescript

XMLWordPrintableJSON

    • Type: Task
    • Resolution: Unresolved
    • Priority: Major - P3
    • None
    • Affects Version/s: None
    • Component/s: None
    • 3
    • None
    • Developer Tools

      Use Case

      As a maintainer of @mongodb-js/mql-typescript
      I want js-yaml upgraded from ^4.1.1 to ^5.2.2 in packages/mql-typescript
      So that the package stays on a supported js-yaml major.

      User Experience

      • No user-facing change is expected. @mongodb-js/mql-typescript ships generated type definitions (out/schema.d.ts), and js-yaml is a devDependency used only by the code generator to read the MQL specification fixtures. Consumers should see a byte-identical generated schema.
      • The observable outcome for maintainers is that npm run generate-schema, generate-tests and generate-driver-schema keep working unchanged.

      Risks/Unknowns

      • js-yaml 5 removes the entire custom-type API that src/generator.ts is built on: the yaml.Type constructor and DEFAULT_SCHEMA.extend() are gone, as are the per-type kind, construct, predicate and instanceOf options. The replacement is yaml.defineScalarTag() / yaml.defineSequenceTag() composed onto a schema via Schema.withTags(), with construct becoming resolve and predicate/instanceOf collapsing into identify. This is a hand-written migration of 8 custom tags, not a version-number change.
      • The custom tags in scope are !bson_utcdatetime, !bson_objectId, !bson_uuid, !bson_regex (registered twice — once scalar, once sequence), !bson_binary, !bson_decimal128 and !bson_int64.
      • The sequence form of !bson_regex is the most awkward case. Today it destructures construct([pattern, flags]) in one shot; js-yaml 5 sequence tags use an incremental create / addItem / finalize carrier protocol, so the two-element [pattern, flags] contract has to be reconstructed in finalize and will silently misbehave if a fixture ever supplies a different arity.
      • GeneratorBase's constructor currently reaches into yaml.DEFAULT_SCHEMA.implicit and mutates the built-in Date type — swapping instanceOf: Date for a predicate — so that BsonDate (a Date subclass) is not captured by the default Date representer. That internal array does not exist in js-yaml 5 and there is no direct equivalent. We need to decide whether the disambiguation is still needed at all: it only affects dump/represent behaviour, and this generator only ever loads YAML.
      • Schema breadth is a real trap. If the migration builds on CORE_SCHEMA rather than the old DEFAULT_SCHEMA, it silently drops the non-core implicit tags — !Unable to render embedded object: File (timestamp}}, {{) not found.Unable to render embedded object: File (merge}}, {{) not found.Unable to render embedded object: File (binary}}, {{) not found.Unable to render embedded object: File (set}}, {{) not found.Unable to render embedded object: File (omap}}, {{) not found.!pairs. Any of these appearing in an existing fixture would change from "parsed" to "unresolved tag". Confirm which are actually used before narrowing the schema.
      • @types/js-yaml should be removable, since js-yaml 5 ships its own type declarations. Worth confirming depcheck stays happy.
      • Unknown: whether js-yaml 5's stricter tag resolution rejects any fixture currently accepted by 4.x. There are ~thousands of YAML definitions under mql-specifications/, and a resolution regression would surface as a generation failure or, worse, as a quietly different generated type.

      Acceptance Criteria

      Implementation Requirements
      • Bump js-yaml to ^5.2.2 in packages/mql-typescript/package.json and drop @types/js-yaml.
      • Port all 8 custom BSON tags in src/generator.ts to defineScalarTag / defineSequenceTag + Schema.withTags().
      • Resolve the BsonDate vs Date disambiguation explicitly — either reimplement it under the new API or remove it with a comment justifying why load-only usage makes it unnecessary.
      • Choose the base schema deliberately and document the choice, including which non-core tags are intentionally included or excluded.
      • Keep the change scoped to this one package and this one concern. No drive-by formatting, and no broadening of lint suppressions (e.g. avoid adding a file-wide eslint-disable to work around a new type signature — narrow the suppression to the offending line or fix the type).
      Testing Requirements
      • Regenerate all three outputs and confirm a zero-diff against the current committed artifacts: npm run generate-schema, npm run generate-tests, npm run generate-driver-schema. A clean git diff on out/, tests/ and mql-specifications/definitions is the primary signal that the tag migration is behaviourally equivalent. Note that Prettier reformatting can mask this — regenerate against the same Prettier version that produced the committed files, or review any diff line by line to prove it is formatting-only.
      • npm run test (typecheck of the generated tests/*/.ts) passes.
      • npm run check passes — typecheck, eslint, Prettier and depcheck.
      • Add or extend coverage that actually exercises each custom tag through yaml.load, asserting the concrete BSON type produced (e.g. !bson_int64 yields a bson.Long with the expected value). The scalar and sequence forms of !bson_regex both need a case, since they are separate code paths. Today the tags are only covered indirectly via full-schema generation, which makes a partial regression hard to localise.
      • Confirm no fixture under mql-specifications/ fails to parse — generation completing without error across the full definition set is the check here.
      References
      • js-yaml 5 release notes / migration guide for the Type to tag-definition API change: https://github.com/nodeca/js-yaml/releases
      • Affected source: packages/mql-typescript/src/generator.ts (GeneratorBase.loadOptions and the constructor) and packages/mql-typescript/src/driverSchema/driver-schema-generator.ts
      • Context: this was originally attempted as part of a bulk npm update dependency catch-up branch (deps-catchup-minor-patch), which is being split apart because a breaking API migration should not land inside a routine version-bump PR.

            Assignee:
            Unassigned
            Reporter:
            Neal Beeken
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated: