Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load CJS instead of ESM #157

Open
rtritto opened this issue Feb 17, 2023 · 6 comments
Open

Load CJS instead of ESM #157

rtritto opened this issue Feb 17, 2023 · 6 comments

Comments

@rtritto
Copy link
Contributor

rtritto commented Feb 17, 2023

Description

Instance of parsed ObjectId, using ejson-shell-parser (version >= 1.2.1), is different from bson.ObjectId instance (bson version >= 5.0.0-alpha.0).
Correctly work:

  • CommonJS (type: commonjs) with bson v4 and v5
  • ESM (type: module) with bson v4

Same problem with Timestamp, BSONSymbol, DBRef, MinKey, MaxKey.

Steps to reproduce:

  • yarn init -y
  • yarn set version canary
  • yarn add bson ejson-shell-parser
  • Add "type": "module" in pakcage.json
  • Create index.js file
import { ObjectId } from 'bson' 
import parser from 'ejson-shell-parser'
const out = parser.default('ObjectId()')
console.log(out instanceof ObjectId)
  • yarn node index.js

Versions:

bson >= 5.0.0-alpha.0
ejson-shell-parser >= 1.2.1
Node: 16.17.0

Actual

log is false

Expect

log is true

@rtritto rtritto changed the title Parsed ObjectId instance is different from bson.ObjectId one (ESM) Parsed instance is different from bson type one (ESM) Feb 17, 2023
@rtritto
Copy link
Contributor Author

rtritto commented Feb 22, 2023

const out = parser.default('ObjectId()')
console.log(out instanceof ObjectId)  // false, should be true
console.log(typeof out)  // object
console.log(out.constructor.name)  // ObjectId

const outBson = new ObjectId()
console.log(outBson instanceof ObjectId)  // true
console.log(typeof outBson)  // object
console.log(outBson.constructor.name)  // ObjectId

console.log(typeof out === typeof outBson)  // true
console.log(out.constructor.name === outBson.constructor.name)  // true

@rtritto
Copy link
Contributor Author

rtritto commented Feb 22, 2023

Implementation of instanceof:

function instanceOf(object, constructor) {
    while (object !== null) {
        if (object === constructor.prototype) {  //object is instanceof constructor
            return true
        } else if (typeof object === 'xml') {  //workaround for XML objects
            return constructor.prototype == XML.prototype
        }
        object = object.__proto__  //traverse the prototype chain
    }
    return false  //object is not instanceof constructor
}

Related problem:

console.log(out.__proto__ === ObjectId.prototype)  // false, should be true
console.log(outBson.__proto__ === ObjectId.prototype)  // true

Imply:

console.log(out.__proto__ === outBson.__proto__)  // false, should be true

@addaleax
Copy link
Contributor

This just seems to be happening because ejson-shell-parser is loading the CommonJS version of bson and you are importing the ESM version of bson.

It’s expected that these are different. You can fix this by manually loading the CommonJS version from your code as well (if you ensure that the package gets deduplicated during yarn/npm install).

Generally speaking, this package doesn’t make guarantees about the specific prototypes that its returned values have; it only returns bson objects from a bson@5.x version.

@rtritto
Copy link
Contributor Author

rtritto commented May 18, 2023

Can I do a PR to covert this to ESM?

Edit:
Maybe it's not needed because dist/ejson-shell-parser.esm.js in package.json

@rtritto
Copy link
Contributor Author

rtritto commented May 18, 2023

ejson-shell-parser should use ESM ~ dist/ejson-shell-parser.esm.js (source package.json), so why?

@rtritto
Copy link
Contributor Author

rtritto commented Jul 28, 2023

@addaleax I imported the ESM version (removing default from parse function) and it works.

import { ObjectId } from 'bson' 
import parser from './ejson-shell-parser.esm.js'  // same as file in dist folder
const out = parser('ObjectId()')
console.log(out instanceof ObjectId)  // output is true

I can confirm that ejson-shell-parser is imported as CJS instead of ESM.
How can ESM be correctly imported?

Edit:

This can be fixed using *.esm.mjs with dual package

Diff in package.json:

-   "main": "dist/ejson-shell-parser.cjs.js",
+   "main": "dist/ejson-shell-parser.cjs",
-   "module": "dist/ejson-shell-parser.esm.js",
+   "module": "dist/ejson-shell-parser.mjs",
...
+   "exports": {
+     "import": "./dist/ejson-shell-parser.mjs",
+     "require": "./dist/ejson-shell-parser.cjs"
+   }
import { ObjectId } from 'bson' 
import parser from 'ejson-shell-parser.esm.js'
const out = parser('ObjectId()')
console.log(out instanceof ObjectId)  // output is true

I opened preconstruct/preconstruct#576

@rtritto rtritto changed the title Parsed instance is different from bson type one (ESM) Load CJS instead of ESM Jul 29, 2023
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

No branches or pull requests

2 participants