MunifTanjim / node-bitbucket

Bitbucket API client for Browser and Node.js
https://bitbucketjs.netlify.app
MIT License
106 stars 28 forks source link

Add support for --isolatedModules #53

Closed mishkinf closed 4 years ago

mishkinf commented 4 years ago

This library doesn't compile in typescript projects that use isolatedModules=true in their tsConfig.js such as a Create React App. The reason being is because types are shared across several of the defined packages. Sharing the types prevents the compiler from isolating the modules.

This PR addresses that issue by importing the types and re-exporting them by using typescript's new type only import / exports and re-exports types so modules can be isolated.

Refer to https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html

Unfortunately Prettier does not support typescript 3.8 yet so I have disabled prettier in this PR for now as well. Refer to https://github.com/prettier/prettier/issues/7263

The error looks like:


/Users/blah/workspace/project/node_modules/bitbucket/src/client/types.ts
TypeScript error in /Users/blah/workspace/project/node_modules/bitbucket/src/client/types.ts(6,10):
Re-exporting a type when the '--isolatedModules' flag is provided requires using 'export type'.  TS1205

    4 | import { Request, Response } from '../request/types'
    5 |
  > 6 | export { EndpointParams } from '../endpoint/types'
      |          ^
    7 |
    8 | export interface Options {
    9 |   [option: string]: any
MunifTanjim commented 4 years ago

I don't think jumping to TypeScript 3.8 would be a good idea. :thinking: Many people are still stuck with older TypeScript versions.

mishkinf commented 4 years ago

Sure. I just can’t use this repo without the change. I may have to cut a separate release for my own needs.

Another solution would be to change how project is broken into modules but it was more involved than I wanted to go for my own needs.

MunifTanjim commented 4 years ago

Hey @mishkinf,

Sorry for the late reply.

Can you please change all of the type imports & exports like this in the types.ts files?

// this is just imported (can be used in this file)
type APIClient = import('../../client/types').APIClient

// this is both imported (can be used in this file) & exported (can be imported from another file)
export type APIClient = import('../../client/types').APIClient

This should solve the problem with isolatedModules=true and we won't need TS 3.8.

PetrShchukin commented 4 years ago

When the pull request is gonna be merged? Have the same problem with typescript

mishkinf commented 4 years ago

Based on what MunifTanjim has said as the owner, which makes sense, it is unlikely to get merged as it is now. However there is a need for somebody to restructure this project in a typescript friendly way the way it described by him in his comment on March 25. If you or anybody would like to take that on please do! You can also fork my branch and publish your own package if you'd like..

PetrShchukin commented 4 years ago

So far I can't even make it work with React.. As I need some little piece of functionality I think I will go with bitbucket rest api

MunifTanjim commented 4 years ago

Thanks @mishkinf for the initial PR. I've refactored and merged it.

@PetrShchukin this will be published shortly.

This also resolves #61

mishkinf commented 4 years ago

thank you @MunifTanjim ! all those typescript / create react app people will be happy out there. :)

PetrShchukin commented 4 years ago

Cheers!