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 only imports 166 #210

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

download13
Copy link
Collaborator

Fixes #166

Type imports in guard files can be type-only imports, which some build tools need to know which import statements to drop for runtime.

Code change is only one line, but test changes involve all test guards using type-only imports.

Incompatible with typescript versions prior to 3.8, so this is probably a breaking change.

@download13 download13 added the do not merge This PR is not ready to be merged label Oct 14, 2022
@download13
Copy link
Collaborator Author

Hang on this isn't ready. Got ahead of myself and just noticed the note about #162

@rhys-vdw
Copy link
Owner

@download13 planning to take another look at this at some stage? (no pressure)

@download13
Copy link
Collaborator Author

@rhys-vdw Yeah I kinda wandered off and forgot about it :P

Looking back at my branch I realize why I stopped working. I think at the time the versions of typescript and ts-morph in the deps were quite old, and I ran into some errors while trying to update them.

Now that those have been updated, and once #215 is merged, I should be able to use mixed type/value imports to finish off #166.

@rhys-vdw
Copy link
Owner

Awesome. Merged and ready!

@PierreMardon
Copy link

I really enjoy auto-guard, at the point I manually edit the imports to match the type imports requirements of my setup 😅
So... just pinging innocently 😇

@warna720
Copy link

Yep, looking forward to this being released 👍

@rhys-vdw
Copy link
Owner

I'm sure @download13 wouldn't mind if someone forks his branch and continues the work (if you're really keen)

@fthdgn
Copy link
Collaborator

fthdgn commented Oct 21, 2023

I need this because of "verbatimModuleSyntax": true on tsconfig.

For this use case, isTypeOnly: project.compilerOptions.get().verbatimModuleSyntax === true can be used, instead of isTypeOnly: true.

However verbatimModuleSyntax exists only if ts-morph version is larger then 19.0.0. And this version breaks generated_type_guards_with_a_short_circuit_are_correctly_stripped_by_UglifyJS.test.ts.

Using verbatimModuleSyntax ensures that ts-auto-guard can be used on typescript versions older than 3.8, because these versions cannot make verbatimModuleSyntax === true

@rhys-vdw
Copy link
Owner

@fthdgn I'm more than happy to bring you on as a collaborator if you're interested in making this happen. I'm not a regular typescript programmer anymore, let alone a consumer of this project.

@fthdgn
Copy link
Collaborator

fthdgn commented Oct 22, 2023

@rhys-vdw
I've received your invitation. I will accept it. Thank you. I will try to help the project on my free times.

@rhys-vdw
Copy link
Owner

@rhys-vdw I've received your invitation. I will accept it. Thank you. I will try to help the project on my free times.

No pressure of course, but I'll be here to review, merge and release if you would like to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge This PR is not ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type-only imports
5 participants