-
Notifications
You must be signed in to change notification settings - Fork 508
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
Respect custom tsconfig path for esModuleInterop #436
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this @tricoder42 !! Thought I might've missed something when I fixed the esModuleInterop
setting (#327 ). Would be good to have a test for custom tsconfig
paths to check for these types of bugs.
Could you rename the PR & commit to "fix: respect custom tsconfig path for esModuleInterop" to be more specific?
It's a bit misleading in its current form, as custom tsconfigs are used internally and custom tsconfig path is used internally by rollup-plugin-typescript2
(see this line), but not for the esModuleInterop
setting (which is a more recent feature/fix)
@agilgur5 Both commit and PR were renamed. Thanks for the review! 👍 |
@chancestrickland I'm an active contributor but don't have merge privileges -- would merge & release a patch if I could! If @sw-yx didn't merge I assume he's waiting for Jared's approval? Jared also handles releases. I think you should be able to workaround it by overriding the rollup config with |
@agilgur5 Excellent, thanks for the link and super-quick reply. I'll see if I can go that route for now. 👍 |
yea usually for anything config-centric i want to make sure jared approves everything personally since thats the core of tsdx. i dont want to make decisions that he then has to reverse. he also handles the deploy. so leaving to @jaredpalmer. dont forget you can also do local forks while u wait for pr's to get merged, with http://npm.im/patch-package |
I genuinely did not know about |
For sure. I thought one-liner bugfixes like this might fall into a different category since they're not really changing any behavior tho (I feel like I've made a few PRs like that but idr) |
Sorry for delay. Will get this out now |
@all-contributors please add @tricoder42 for bug, code |
I've put up a pull request to add @tricoder42! 🎉 |
Hmmm, so after writing #526 , I wonder if this will work properly without using |
When custom
tsconfig
is used (e.g.tsdx build --tsconfig tsconfig.build.json
), it is ignored increateRollupConfig
.For example, this causes bug in reach-ui build reach/reach-ui#434. They use
tsconfig.build.json
, which is being ignore and the build files are missingObject.defineProperty(exports, '__esModule', { value: true });
.