Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upMigrate tsickle to TypeScript 4.0. #1165
Conversation
| @@ -11,7 +11,7 @@ | |||
| "src/*" | |||
| ], | |||
| "peerDependencies": { | |||
| "typescript": "~3.9.5" | |||
| "typescript": "^4.0.0-dev" | |||
This comment has been minimized.
This comment has been minimized.
evmar
Jul 22, 2020
Collaborator
We should use patterns like ~4.0 so that we don't implicitly pull in e.g. 4.1.
This comment has been minimized.
This comment has been minimized.
mprobst
Jul 22, 2020
Author
Collaborator
I went with ~4.0.0-dev so that we get the dev packages and patch levels in 4.0, but not 4.1
| // as otherwise TS fails when resolving types for decorators. | ||
| sf = ts.getMutableClone(sf); | ||
| sf.statements = statements; | ||
| sf = ts.factory.updateSourceFile( |
This comment has been minimized.
This comment has been minimized.
| const s = await 3; | ||
| return s + this.field; | ||
| return s + (this as any).field; |
This comment has been minimized.
This comment has been minimized.
evmar
Jul 22, 2020
Collaborator
I think this isn't right, and might break dis/ambguation.
I can't tell from this change if it's meant to be a workaround, or if we expect user code to change in the same way?
This comment has been minimized.
This comment has been minimized.
mprobst
Jul 22, 2020
Author
Collaborator
Good catch. I was playing around with this and didn't remove the as any cast again. This ultimately didn't need a change I think.
This comment has been minimized.
This comment has been minimized.
mprobst
Jul 22, 2020
Author
Collaborator
The tricky bit is that TS compiler now disallows having a function without the this type specified here, so I needed to add an explicit this param type. That means a code path in tsickle is now under-exercised (never have a nested fn that needs a this * added), but I think that's OK.
|
LGTM. Should we get this working internally first, then sync it out from there? I think if this is merged and intermediate changes to tsickle will overwrite these changes. |
|
@rrdelaney we can keep it in this branch for the time being. I'll create an internal CL that we can patch to test the new version. |
This mostly involves adjusting a number of callsites to the new `ts.factory` API instead of the previous `ts.getMutableClone`. Additionally it required several test changes: - an unspecified global this no longer is any - properties may no longer override getters - stricter checks of property initialization
mprobst commentedJul 22, 2020
This mostly involves adjusting a number of callsites to the new
ts.factoryAPI instead of the previousts.getMutableClone.Additionally it required several test changes: