Skip to content

Conversation

go1t
Copy link
Contributor

@go1t go1t commented Feb 2, 2022

We have configured baseUrl in tsconfig.json, which allows absolute imports to be resolved by typescript. However, we didn't configure the same thing in the rollup config, specifically in the dts plugin. This led to an issue after the package was published #372. This PR adds a baseUrl configuration to ensure that absolute imports can be resolved the same way during the rollup process.

Note that alternatively we could also remove baseUrl from tsconfig.json to ensure that we always use relative imports too. In fact, most of the code is already using relative imports. If you prefer this, please let me know and I can close this PR and make another one

Fly-bys:

  • Add back the previous changes in Offers and OfferRequests
  • Stop excluding spec files in tsconfig.json and remove @ts-ignore

@go1t go1t requested a review from a team as a code owner February 2, 2022 23:25
@go1t go1t requested review from shaundon, mcdeguzman99, lapa182 and a team and removed request for a team February 2, 2022 23:25
Copy link
Contributor

@lapa182 lapa182 left a comment

Choose a reason for hiding this comment

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

Thanks, @go1t! Just to let you know, build: won't trigger a new release to NPM, so I would say to amend your commit with fix and add a scope named types just so that we trigger a new release with the type fix.

@go1t
Copy link
Contributor Author

go1t commented Feb 3, 2022

@lapa182 ah I see. thank you!

We have configured `baseUrl` in tsconfig.json, which allows absolute imports to be resolved by typescript. However, we didn't configure the same thing in the rollup config, specifically in the `dts` plugin. This led to an issue after the package was published #372. This PR adds a baseUrl configuration to ensure that absolute import can be resolved the same way during the rollup process.
@go1t go1t force-pushed the remove-baseUrl-tsconfig branch from b50bfbc to 7168e59 Compare February 3, 2022 15:46
@go1t go1t changed the title build: make rollup's dts plugin work with tsconfig's baseUrl fix(types): make rollup's dts plugin work with tsconfig's baseUrl Feb 3, 2022
@go1t go1t merged commit 77f764e into main Feb 3, 2022
@go1t go1t deleted the remove-baseUrl-tsconfig branch February 3, 2022 15:50
@github-actions
Copy link

github-actions bot commented Feb 3, 2022

🎉 This PR is included in version 1.7.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants