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 type mapping #72

Closed
wants to merge 6 commits into from
Closed

Fix type mapping #72

wants to merge 6 commits into from

Conversation

Gozala
Copy link
Contributor

@Gozala Gozala commented Mar 30, 2021

This is based of #70 pull request. And resolves the issue described here #66 (comment). There is also relevant thread here ipld/js-car#7

Overview of changes

  • Scripts are changed to use newer tsc --bulid command (they use different code path and other ones will eventually be removed)
  • Prepare script was added so that other package installs this as git dependency types will be generated in right location.
  • Works around TS bug typeVersions substitution occurs multiple times causing resolution to fail microsoft/TypeScript#41284, which causes it to do path substitution twice when types field is present. That causes multiformats to be rosolved to node_modules/multiformats/types/index.d.ts and then with node_modules/multiformats/types/types/index.d.ts. Here we just map types/* back to types/* which prevents second substitution and avoids the mentioned bug.

I really wish things worked without all this hacks, but 🤷‍♂️

vmx and others added 6 commits March 29, 2021 10:38
It also removes the `Codec` class and exports a plain object
instead. This has consequences for imports.

BREAKING CHANGE:

Only use names exports as default exports don't play well with
CommonJS + Typescript typing.

This means that when you use ESM imports, e.g. the raw codec is
no longer imported as

```js
import raw from 'multiformats/codecs/raw'
```

but as

```js
import * as raw from 'multiformats/codecs/raw'
```

The CJS import for codecs don't change, it's still `const raw = require('multiformats/codecs/raw`.

Though other imports change, so

```js
import CID from 'multiformats/cid'
const CID = require('multiformats/cid')
```

is now

```js
import { CID } from 'multiformats/cid'
const { CID } = require ('multiformats/cid')
```
- "*" match seems to cause multiple path substitutions
microsoft/TypeScript#41284 that packge name prefix seems to resolve
- add types field which acts as main in TS
- modify scripts to use newer tsc --bulid command
- generate types on prepare so linking to git dependency works
- workaround douple substitution by mapping types/* to itself
   (see microsoft/TypeScript#41284)
@rvagg
Copy link
Member

rvagg commented Mar 31, 2021

👍 amazing detective work, types/* -> types/* is whacky, but it works!
pulled in to named-exports

@rvagg rvagg closed this Mar 31, 2021
@rvagg rvagg deleted the fix/type-mapping branch March 31, 2021 03:55
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.

3 participants