Skip to content

Update Typescript to v5 #711

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

Update Typescript to v5 #711

wants to merge 3 commits into from

Conversation

janpe
Copy link

@janpe janpe commented Apr 4, 2023 •
edited
Loading

Update Typescript to ^5.0.0

createTransformer requires some refactoring regarding transformer return value.

@janpe janpe mentioned this pull request Apr 4, 2023
Closed
@Igorbek
Copy link
Owner

Igorbek commented Apr 4, 2023

It seems the version of the node in the GitHub actions has to be updated to >=12.20.

@janpe
Copy link
Author

janpe commented Apr 5, 2023 •
edited
Loading

It seems the version of the node in the GitHub actions has to be updated to >=12.20.

Where are these pipelines configured? Can't seem to find anything about them in the repository. Running in Azure, not Github?

@quantizor
Copy link
Contributor

quantizor commented May 18, 2023 •
edited
Loading

It might be easier to just switch to normal Github actions? I could whip up a PR for that @Igorbek if you're interested whipped it up: #712

You might need to change a setting to allow forked repos to run workflows so it shows up in the PR prior to merging

Igorbek
Igorbek requested changes May 19, 2023
View reviewed changes
Copy link
Owner

@Igorbek Igorbek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be reasonable to keep the support of 4.x along with the new 5.x

package.json
@@ -21,7 +21,7 @@
},
"typings": "dist/index.d.ts",
"peerDependencies": {
"typescript": "^4.0"
"typescript": "^5.0.0"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it'd be nice to keep 4.x support 4 || 5

src/createTransformer.ts
@@ -261,7 +261,7 @@ export function createTransformer({
);

const updateExportAssignmentExpression = (node: ts.ExportAssignment, expression: ts.Expression) =>
context.factory.updateExportAssignment(node, node.decorators, node.modifiers, expression);
context.factory.updateExportAssignment(node, node.modifiers, expression);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to support both - old and new APIs? maybe pass TS version parameter to the config

@Igorbek
Copy link
Owner

Igorbek commented May 19, 2023

@janpe please take a look - the tests are failing now
now you can see the tests in the GH actions (thanks to @probablyup )

@janpe
Copy link
Author

janpe commented May 30, 2023

@janpe please take a look - the tests are failing now now you can see the tests in the GH actions (thanks to @probablyup )

I'm currently not able to find time to proceed with this

@Igorbek Igorbek mentioned this pull request Jun 2, 2023
Merged
Igorbek added a commit that referenced this pull request Jun 2, 2023
@Igorbek
Update TypeScript to 5.1 and support TS 4.8+ (#713)
2daa5df 
fixes #710, supersedes #711

- the peer dependency is set to 4.8+ because there [was a breaking
change in the API we use in
4.8](https://github.com/microsoft/TypeScript/wiki/API-Breaking-Changes#decorators-are-placed-on-modifiers-on-typescripts-syntax-trees)
- use `styled-components` package for types instead of
`@types/styled-components`
@Igorbek
Copy link
Owner

Igorbek commented Jun 2, 2023

superseded by #713

@Igorbek Igorbek closed this Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Igorbek Igorbek Igorbek requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@janpe @Igorbek @quantizor