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

fix: Fix types and typescript config for publishing #162

Merged
merged 9 commits into from
Aug 14, 2023

Conversation

gmaclennan
Copy link
Member

@gmaclennan gmaclennan commented Aug 10, 2023

Several type-related tasks to prepare for publishing to npm:

  • Fixed up a bunch of types
  • Updated tsconfig to use Node16 module resolution (with related fixes) - necessary since we are publishing this as an ESM module.
  • Created a separate tsconfig.json for building declaration files for npm publishing
  • Updated npm exports and which files are included in the npm package
  • Updates drizzle for simplification of typescript definitions - pre-v0.28.0 the declarations werre 57Mb and made tsc hang

TODO:

  • When consuming this as an npm module the types aren't quite right - project.observation.getByDocId() returns { [string]: any }, even though it works from within the e2e tests. Something to do with declaration compilation. Going to simplify the types of DataType to see if that works.

@gmaclennan gmaclennan self-assigned this Aug 10, 2023
Copy link
Member

@achou11 achou11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

what from Drizzle was causing the massive output?

tsconfig.json Show resolved Hide resolved
@achou11
Copy link
Member

achou11 commented Aug 10, 2023

ah shoot didn't realize this was still a draft 😅 consider my approval an incremental one haha

Base automatically changed from chore/code-cleanup to main August 10, 2023 13:26
Node16 moduleResolution is stricter, and there is a bug in typeRoots
module resolution, so need to use `paths` config to resolve custom types
in @digidem/types.

Because of stricter resolution, Typescript no longer likes merging types
from both a `.js` and `.d.ts`. Switched `types.js` and `types.d.ts` to
be a single `.ts` file that defines the types. Using `.d.ts` for this
causes issues when compiling declarations for module packaging.
Using package.files instead of `.npmignore`
Needed to update drizzle for simplification of typescript docs
@gmaclennan gmaclennan changed the title fix: Fix drizzle types fix: Fix types and typescript config for publishing Aug 10, 2023
@gmaclennan gmaclennan force-pushed the fix/typescript-fixes-and-config branch from 6262a95 to 920a516 Compare August 10, 2023 13:35
@gmaclennan gmaclennan linked an issue Aug 10, 2023 that may be closed by this pull request
3 tasks
@gmaclennan
Copy link
Member Author

Current status: types work fine within the module (e.g. when type-checking the e2e tests everything is correct) but when using via npm pack packed.tgz and then npm i ../mapeo-core-next/packed.tgz the types aren't been read correctly - project.observation.getByDocId() doesn't return the correct type.

This seems to be related to a whole jumbled TS definition in the declaration file for src/datatype/index.d.ts. Will investigate further, and try simplifying some of the types on the constructor to try and get the method types working correctly.

Typescript was not correctly compiling the declaration,
so switched to write it manually.
Also includes e2e tests in `npm test`
@gmaclennan gmaclennan marked this pull request as ready for review August 14, 2023 11:11
@gmaclennan
Copy link
Member Author

Have switched to manually writing the .d.ts file for DataType. Not an ideal solution, but it ensures that the exported types are correct. @achou11 could you take another look over this?

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.

Prepare module for publishing alpha on npm
2 participants