2.1 beta

[nonara]

Summary

• Refactored & reorganized code
• Ensured that we use the passed typescript instance (as opposed to locally resolved typescript) (Fixes Plugin uses module 'typescript' instead of the instance instantiating the plugin #80)
• Implemented proper type elision Fixes ttsc --watch not working when upgrade typescript-transform-paths to 2.0.3 #78 Fixes Workaround for Import/Export Declarations can break chained transformers #77 (detail below)
• Dropped travis config
• Cleaned up and trimmed down readme
• [Test-related]
  • Added testing for multiple TS versions
  • Testing both ts-patch and ttypescript
• Switched to yarn (faster, smaller footprint, and necessary for aliasing used with multi-TS packages)

Note: No official minimum version was set, so I've set it to 3.6.5 and we're testing using that version as well as the latest, which covers the old and new factory style API.

Update on Type-Elision Debacle

I spent some time in the TS compiler codebase and was able to dialog a bit with one of the MSFT TypeScript team members, who was kind enough to point me in the right direction.

Initially, I was planning to do a PR for TypeScript, but it turns out that because of the way type elision is written, the decision on how or if to move forward ultimately falls to the TS team.

The good news, however, is that I was able to locate the code which performs type elision and determine why it doesn't work for updated nodes. This lead to determining a route for recreating the behaviour in a way that will not circumvent the API.

Relevant changes:

• utils/get-import-export-clause.ts
• visitor.ts

ToDo

• Work out final kink with type elision

[nonara]

Hi @danielpza ! Lots of edits - but good ones ☺

Refactoring

I realized after adding the elision code, that the file size had grown a bit more than is ideal for a single file, so a refactor seemed like a good idea. You can see a summary above of what was done. The refactoring was to make the code easy to read and manage.

If you want to pull the latest and have a look, I think you'll find that it should be easy to maintain moving forward.

Test Refactors

Regarding tests, I refactored the structure to be more modular to allow for adding specific unit testing and more easily adding new project dirs & more versions of TS as needed. Overall, the changes should make it easier to scale and add testing in the future should we need to.

Merging

Because there are a few changes, I wanted to wait on your input before merging.

If you're busy, here's a short list of the things I'd love to get some feedback on:

1. Is it ok to drop travis config?
2. Do we need appveyor? If so, we'll need to drop node 8 as prettier is complaining.
3. Does the new readme look okay?

The goal of the readme was to organize, remove redundancy, and simplify. I also added a maintainers section.

Looking forward to your thoughts!

[nonara]

Note: There are a couple of odd issues with the current implementation. I was hoping to lean on the API for the import/export clause rather than re-create the logic it uses, but it looks like this may not be possible.

For now, I'm going to go ahead and recreate the elision functionality found in TS: src/compiler/transformers/ts.ts.

[danielpza]

Thanks, I'll be taking a look in the next few days

[danielpza]

Is it ok to drop travis config?

Yes.

Do we need appveyor? If so, we'll need to drop node 8 as prettier is complaining.

We had a trouble once with paths on windows. That's why we included appveyor, It should be fine to drop node 8 support

Does the new readme look okay?

Yes, it does.

[danielpza]

@nonara great work on this, as always :)

Could you provide some info on Type-Elision? perhaps link to some related discussions if they were public?

[nonara]

@nonara great work on this, as always :)

Thanks! I appreciate that!

Could you provide some info on Type-Elision? perhaps link to some related discussions if they were public?

I don't think that there's anything official written up about it. It's one of the areas of the compiler that you'd need to know what you're looking for, which is why I'm grateful for Andrew Branch's help on pointing me to the right area!

This thread should be very informative on that: microsoft/TypeScript#40603

Also, you can have a look at the code /src/compiler/transformers/ts.ts, which is pretty easy to follow and track down its behaviour.

As a quick summary - what it's basically doing is checking the Symbol links to see if each imported / exported element is referenced somewhere.

So, for example, if we import three things A, B, and C, the symbol has information on whether those imports are referenced anywhere. If not, they're elided from the export.

If we say B is a const that it used in the file, while A & C are types, it sees that only B is referenced in the JS output, therefore, the import declaration is output with only B.

Alternatively, if all three are types, it will elide the entire import declaration, as the original node had an ImportClause that is now empty.

What's happening for us is that whenever you update a node during transform, it effectively creates a brand new node, with the original property set to the one which preceeded it. The TS compiler is set to skip all elision functionality if it sees that it is not working with the original node. Andrew goes into the reasoning behind this in the thread above. It certainly does make sense, in the sense that chaining transformers can create some difficult situations. That's why there needs to be a determination on their end of how to handle that.

[nonara]

We had a trouble once with paths on windows. That's why we included appveyor, It should be fine to drop node 8 support

Interesting! If you'd like to consolidate to a single CI, I believe we can do this:

jobs:
  build:
    runs-on: ${{ matrix.os }}
    strategy:
      matrix:
        os: [ubuntu-latest, windows-latest]
        node-version: [10.x, 12.x, 14.x]

[danielpza]

Interesting! If you'd like to consolidate to a single CI, I believe we can do this:

hmm, yeah, that would be better, didn't know about that.

[nonara]

Should be all sorted!

• I extracted valid parts of the TS source for elision.
  It's small and simple, so there's not too much risk for divergence. Assuming TS does make a change in the future, we'll be able to implement the new method for TS 4.X+ and keep the old for prior versions

• Created new HarmonyFactory, which cleaned up the visitor code a lot!
  Basically it provides a routing device for old / new TS factory that lets us use the latest syntax, and downgrade if needed

I see appveyor is still failing. Is it setup somewhere in GitHub or elsewhere ? Or maybe it's just set to use the previous config until the deleted config file is known on master.

Otherwise - this is ready. Let me know if there's anything you'd like to address, and when you're good, I'll publish!

[danielpza]

I see appveyor is still failing. Is it setup somewhere in GitHub or elsewhere ? Or maybe it's just set to use the previous config until the deleted config file is known on master.

I just removed some appveyor config, I think next builds wont use appveyor anymore.

Otherwise - this is ready. Let me know if there's anything you'd like to address, and when you're good, I'll publish!

2.1 beta #81 consider following the semantic version specs for the merge commit, other than that I think is good so go ahead 💯

[nonara]

2.1 beta #81 consider following the semantic version specs for the merge commit

Ha. Yeah sorry about that last commit message, I realized it just after hitting the button. This will follow spec.