Skip to content

Proper import/export filter #14

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

Closed
wants to merge 1 commit into from
Closed

Proper import/export filter #14

wants to merge 1 commit into from

Conversation

anion155
Copy link
Contributor

@anion155 anion155 commented May 21, 2019

Have found the way ts filters declarations. Did replicate it. Source.

@danielpza
Copy link
Member

@anion155, these are a lot of changes, care to explain the process?

@anion155
Copy link
Contributor Author

anion155 commented May 21, 2019

Yep. I really wanted to discuss this, but it's hard without the code. So it's more like suggestion.

Manually changing text is not really good solution, types-only exports not filtered as in vanilla ts, which makes them stay in result file as import side effects.

So, I did search how this module declarations handled in compiller. As you can see in previous link it is made by transformer, so I just copied filter logic over here.

But I can't keep silent about resolver, it is internal api in ts compiller. It could become a problem in future. But it can be easily guarded.

What if leave imports as is it is now (.text =) and make only exports use resolver, only when resolver is available?
About extensions. I needed to check if module exists, and for I used fs.exists for which I needed extensions, project using custom loaders or transformers, which could load not only js modules, but and images and css and anything. So I made extensions customizable. But I recently found about require.resole function, so I will probably rewrite this a bit later. It had to be probably in standalone pr.

@danielpza
Copy link
Member

What if leave imports as is it is now (.text =) and make only exports use resolver, only when resolver is available?

What's the difference between the imports and the exports?

@anion155
Copy link
Contributor Author

Types-only re-export does not getting filtered. In tests/utiles/types-only.ts uncomment throw row and tests will fail, cause this module will be imported at runtime.

@danielpza
Copy link
Member

danielpza commented May 22, 2019

Actually that's because of the * syntax in export * from "@utils/types-only";

modified   tests/utils/index.ts
@@ -1,3 +1,3 @@
 export * from "@utils/sum";
 export * from "@utils/subs";
-export * from "@utils/types-only";
+export { NoRuntimecodeHere } from "@utils/types-only";

changing from * to { NoRuntimecodeHere } fixes the issue, so I guess the problem is not between imports and exports but between * and { ImplicitExports }

EDIT: Apparently it only happens with the export * from "" syntax, import * as works

@anion155
Copy link
Contributor Author

Did not thought about that. Will update pr.

@anion155
Copy link
Contributor Author

I've used previous version of visit and added filter of types-only star re-export declarations. And used require.resolve for modules resolving.

@danielpza
Copy link
Member

@anion155, there are some conflicts with the other branches.

Also, could we keep the output of the tests in a separate directory like before in out/, I think it's cleaner that way, what do you think?

@danielpza
Copy link
Member

danielpza commented May 24, 2019

@anion155, could you upload the original PR?, github doesn't allow me to fetch a previous commit.
EDIT: nvm, I refloged my copy

danielpza pushed a commit that referenced this pull request May 24, 2019
Used replica of typescript compiler visitor
- fix export * syntax

closes #13, #14
@danielpza
Copy link
Member

closed in #17

@danielpza danielpza closed this May 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants