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

feat: add TypeScript type annotation tests with node 22 snapshots #21

Merged
merged 3 commits into from
Aug 2, 2024

Conversation

mertcanaltin
Copy link
Member

@mertcanaltin mertcanaltin commented Jul 30, 2024

Greetings, I added a test and tried to get more coverage

  • Add test for stripping type annotations from functions
  • Add test for stripping type annotations from classes
  • Add test for stripping type annotations from interfaces
  • Add test for stripping type annotations from type aliases
  • Add test for stripping type annotations from generics
  • Add test for stripping type annotations from arrow functions
  • Add test for stripping type annotations from type assertions

Additionally, I have integrated node 22 snapshot testing as suggested. The snapshot tests have been normalized to handle line endings and formatting issues

@marco-ippolito
Copy link
Member

Maybe we can use node 22 snapshots testing?

@mertcanaltin
Copy link
Member Author

Maybe we can use node 22 snapshots testing?

do you think I should integrate it here?

@marco-ippolito
Copy link
Member

Maybe we can use node 22 snapshots testing?

do you think I should integrate it here?

Id give it a try, and we can use node 22 in the CI

@mertcanaltin
Copy link
Member Author

Hi, I converted it to a snapshot

@marco-ippolito
Copy link
Member

missing EOF and some formatting issues

test/snapshot.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@mertcanaltin
Copy link
Member Author

I updated the test structure, it seemed that there was no need for the main index test anymore, so I updated the test commands in the package.json, but if any tests are added, we can update here

@mertcanaltin mertcanaltin changed the title feat: add tests for stripping type annotations from various TypeScrip… feat: add TypeScript type annotation tests with node 22 snapshots Jul 31, 2024
@AugustinMauroy
Copy link

I think we can add test for this code:

type User = {
  name: string;
  age: number;
};

function isAdult(user: User): boolean {
  return user.age >= 18;
}

const justine = {
  name: 'Justine',
  age: 23,
} satisfies User;

const isJustineAnAdult = isAdult(justine);

it will be added to the learn section as an example it would be interesting to test it

test/setup.mjs Outdated Show resolved Hide resolved
@mertcanaltin
Copy link
Member Author

I think we can add test for this code:

type User = {
  name: string;
  age: number;
};

function isAdult(user: User): boolean {
  return user.age >= 18;
}

const justine = {
  name: 'Justine',
  age: 23,
} satisfies User;

const isJustineAnAdult = isAdult(justine);

it will be added to the learn section as an example it would be interesting to test it

I have added a test on this, thanks for the suggestion

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@marco-ippolito
Copy link
Member

this should work

		"test": "node --test --experimental-test-snapshots ./test/*.test.js",
		"test:regenerate": "node --test --experimental-test-snapshots --test-update-snapshots ./test/*.test.js"

@marco-ippolito
Copy link
Member

Ci needs to be run with node 22 and also the engine field with node => 22 in the package.json

@mertcanaltin
Copy link
Member Author

yes it worked as you suggested I tried at 22 🚀

@marco-ippolito
Copy link
Member

marco-ippolito commented Aug 2, 2024

please update the ci.yml to use node 22 and add the engines in the package json and squash 🙏🏼

test/setup.mjs Outdated Show resolved Hide resolved
@marco-ippolito marco-ippolito merged commit bec7047 into nodejs:main Aug 2, 2024
5 checks passed
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.

3 participants