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

Setup correct target/module/moduleResolution settings for TSConfig #36

Merged
merged 8 commits into from Aug 10, 2022

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Aug 8, 2022

This reverts some of the changes from #35

  • Setup correct target/module/moduleResolution settings for TSConfig
  • Had to use a deep import of msw/lib/node/index.js in packages/network/__test__/fetch-test.js in order to get TypeScript to properly typecheck the msw
  • Adds patch-package infrastructure
  • Add a patch for vitest so that we can work around issues with type resolution within vite (fix!: use type module (revert #1411) vitest-dev/vitest#1465)

@gabrielcsapo
Copy link
Collaborator

This change doesn't provide a usable ESM module.

@rwjblue rwjblue changed the title Revert "chore: makes changes to be able to consume as an esm package" Setup correct target/module/moduleResolution settings for TSConfig Aug 8, 2022
@rwjblue
Copy link
Member Author

rwjblue commented Aug 8, 2022

This change doesn't provide a usable ESM module.

Hmm, seems to work for me locally when linked into a consumer package... 🤔

@gabrielcsapo
Copy link
Collaborator

Have you tried linking this to a typescript consumer package?

@rwjblue
Copy link
Member Author

rwjblue commented Aug 8, 2022

Have you tried linking this to a typescript consumer package?

Yep! The same one that you shared a screenshot from in #35.

@rwjblue rwjblue marked this pull request as draft August 8, 2022 20:18
Copy link
Collaborator

@gabrielcsapo gabrielcsapo left a comment

Choose a reason for hiding this comment

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

thanks for checking!

@rwjblue rwjblue force-pushed the revert-esm-changes branch 2 times, most recently from 27c131e to e68ab4b Compare August 10, 2022 20:12
Copy link
Contributor

@hjdivad hjdivad left a comment

Choose a reason for hiding this comment

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

LGTM

Let's add an issue for closing the two hacks when we can, to wit:

  1. remove the vitest patch
  2. remove the private import from msw

@rwjblue rwjblue marked this pull request as ready for review August 10, 2022 21:52
@rwjblue rwjblue merged commit df39bf9 into main Aug 10, 2022
@rwjblue rwjblue deleted the revert-esm-changes branch August 10, 2022 21:53
@rwjblue
Copy link
Member Author

rwjblue commented Aug 10, 2022

Created #42 to track fixing those issues.

@rwjblue rwjblue added bug Something isn't working breaking labels Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants