-
Notifications
You must be signed in to change notification settings - Fork 8
fix type exports #546
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
fix type exports #546
Conversation
@@ -1 +1,2 @@ | |||
export * from './download-center'; | |||
export * from './download-center-config'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a line in mongosh that's importing from '@mongodb-js/dl-center/dist/download-center-config'
which was never great, but now breaks with ts compiler options "module": "node16",
"moduleResolution": "node16"
@@ -25,7 +25,8 @@ | |||
"main": "dist/index.js", | |||
"exports": { | |||
"require": "./dist/index.js", | |||
"import": "./dist/.esm-wrapper.mjs" | |||
"import": "./dist/.esm-wrapper.mjs", | |||
"types": "./dist/index.d.ts" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With new ts compiler options:
src/index.ts:428:5 - error TS7016: Could not find a declaration file for module '@mongodb-js/mongodb-ts-autocomplete'. '/Users/leroux.bodenstein/mongo/mongosh/packages/autocomplete/node_modules/@mongodb-js/mongodb-ts-autocomplete/dist/.esm-wrapper.mjs' implicitly has an 'any' type.
There are types at '/Users/leroux.bodenstein/mongo/mongosh/packages/autocomplete/node_modules/@mongodb-js/mongodb-ts-autocomplete/dist/index.d.ts', but this result could not be resolved when respecting package.json "exports". The '@mongodb-js/mongodb-ts-autocomplete' library may need to update its package.json or typings.
I just changed the others where we had types at the top but not inside exports too. It is a bit of a mixed bag across the packages.
@@ -29,7 +29,7 @@ | |||
"main": "dist/index.js", | |||
"exports": { | |||
"./schema": { | |||
"types": "./dist/schema/schema.d.ts" | |||
"types": "./out/schema.d.ts" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We missed this in the mql typescript PR. dist/schema/schema.d.ts is not a thing. We write the schema to out. So this export is not working when imported from mongosh:
src/collection.ts:99:27 - error TS2307: Cannot find module '@mongodb-js/mql-typescript/schema' or its corresponding type declarations.
99 import type * as mql from '@mongodb-js/mql-typescript/schema';
@@ -25,7 +25,8 @@ | |||
"main": "dist/index.js", | |||
"exports": { | |||
"require": "./dist/index.js", | |||
"import": "./dist/.esm-wrapper.mjs" | |||
"import": "./dist/.esm-wrapper.mjs", | |||
"types": "./dist/index.d.ts" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depending on the module resolution settings you pass to the ts compiler settings it might fall back to the types at the root level or it might not. With node16 rules it now tells you about the types at the root level, but doesn't use it. 🙄 I don't think it affects every package because we don't use them all in mongosh, but I figured for consistency I'm just specifying the types everywhere we specify exports. We already did it in a bunch of packages anyway.
No description provided.