teaful icon indicating copy to clipboard operation
teaful copied to clipboard

Migrate package to TS

Open aralroca opened this issue 2 years ago • 5 comments

https://github.com/teafuljs/teaful/issues/60

It's working in progress. I have put the following comment // @ts-nocheck, but before merging this PR has to be deleted so that all types are corrected.

  • [x] Fix all TypeScript errors + remove // @ts-nocheck comments
  • [x] Fix eslint to format .ts and .tsx files
  • [x] Move types to another file
  • [x] Reduce to less than 1kb

aralroca avatar Dec 05 '21 18:12 aralroca

I think it's a good idea to move type definitions to separate file, then import those types with type-imports. https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html https://levelup.gitconnected.com/improving-babel-support-for-typescript-with-type-only-imports-28cb209d9460

ruslan-byondxr avatar Dec 07 '21 19:12 ruslan-byondxr

I think it's a good idea to move type definitions to separate file, then import those types with type-imports. https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html https://levelup.gitconnected.com/improving-babel-support-for-typescript-with-type-only-imports-28cb209d9460

Agree, we'll do it before merge. Thanks!

danielart avatar Dec 07 '21 19:12 danielart

Cloned this branch. Getting typescript error Screen Shot 2021-12-08 at 11 10 10

To fix it, you have to install @babel/eslint-parser: yarn add eslint @babel/core @babel/eslint-parser -D

Then in your .eslintrc.js add parser property: 'parser': '@babel/eslint-parser',

The file should be:

module.exports = {
  'env': {
    'browser': true,
    'es2021': true,
    'node': true,
  },
  'settings': {
    'react': {
      'version': 'detect',
    },
  },
  'extends': [
    'plugin:react/recommended',
    'plugin:react/jsx-runtime',
    'google',
  ],
  'parser': '@babel/eslint-parser',
  'parserOptions': {
    'ecmaFeatures': {
      'jsx': true,
    },
    'ecmaVersion': 2020,
    'sourceType': 'module',
  },
  'plugins': [
    'react',
    'react-hooks',
    'testing-library',
    'jest',
  ],
  'rules': {
    'require-jsdoc': 0,
    'react/prop-types': 0,
    'prefer-const': 0,
    'linebreak-style': ['error', process.platform === 'win32' ? 'windows' : 'unix'],
    'max-len': [
      'error',
      {'code': 80, 'ignoreStrings': true},
    ],
    'no-restricted-imports': [
      'error',
      {
        paths: [
          {
            name: 'react',
            importNames: ['default'],
            message: 'React default is automatically imported by webpack.',
          },
        ],
      },
    ],
  },
};

Now you will see some minor descriptive errors that you can fix easily:

No need to import 'React' ESLint: 'default' import from 'react' is restricted. React default is automatically imported by webpack.(no-restricted-imports)

Increase max-len to 120, or break lines in index.ts ESLint: This line has a length of 121. Maximum allowed is 80.(max-len)

creotip avatar Dec 08 '21 09:12 creotip

Size Change: -38 B (-1%)

Total Size: 3.52 kB

Filename Size Change
dist/index.js 833 B -9 B (-1%)
dist/index.m.js 846 B -11 B (-1%)
dist/index.modern.js 941 B -4 B (0%)
dist/index.umd.js 899 B -14 B (-2%)

compressed-size-action

github-actions[bot] avatar Mar 16 '22 20:03 github-actions[bot]

The PR is finnally ready for review 🎉 @danielart

All project moved to TypeScript + add tests for types. Now already passed all tests and library size less than 1kb again.

The version to test in your projects is 0.11.0-canary.2

Before merging the PR it would be great to test in an actual TypeScript project that everything is working correctly with 0.11.0-canary.2. I'm sure it's better than version 0.10 because now there are many more types, but it would be good to check it apart from review the code.

Looking forward to the review 😊 let's see if we finally release version 0.11 soon, having moved the whole project to TypeScript! 🤩

aralroca avatar Apr 09 '22 11:04 aralroca