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

Add TypeScript support #11

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Conversation

simply-alliv
Copy link

@simply-alliv simply-alliv commented Oct 20, 2019

After cloning a forked version of your repo and running npm install, there were security vulnerabilities found with the version of lodash your repo was using. So this is the updated repo after running npm audit fix.

Have implemented TypeScript into the repo. Things to note:

  1. I have removed prop-types from code and package.json as requested and replaced it with
    typescript types.

  2. Due to your "minimalistic" TypeScript support requirement. I opted to use @babel/preset-
    typescript. It allows babel to be able to transpile TypeScript syntax so that that ESLint and Babel
    can work well with TypeScript files. Due to this, you'll find that the typescript package is not
    installed as it is not a requirement for the project to work. There are some pros and cons to this
    approach. The pro I know of being that:
    - Babel has plugins that allow you to use JavaScript syntax that is not yet supported by
    TypeScript.

    The cons I know of being that:
    - TypeScript files won't have any type checking since Babel can only transpile the code. Meaning
    that Babel will still compile code even if it has type errors.
    Although I believe that ESLint might be utilised well here to provide some form of type
    checking. But I am not sure how effective and type safe this approach might be yet.
    - Some constructs don't currently compile in Babel, with or without TypeScript:
    1. namespaces
    2. bracket style type-assertion/cast syntax regardless of when JSX is enabled (i.e. writing
    x won’t work even in .ts files if JSX support is turned on, but you can instead write
    x as Foo).
    3. enums that span multiple declarations (i.e. enum merging)
    4. legacy-style import/export syntax (i.e. import foo = require(...) and export = foo)

  3. Add type safety for title prop
    Replaced prop-types with TypeScript types.

Alternatives:
Include typescript as a package and let it do the type checking. Allows for type checking through
continuous integration and other features such as emitting declaration files for the .js files in a
production build.

 Or use the @typescript-eslint package. This is more mature but the configuration is a bit more 
 complex than the @babel/preset-typescript way.

Personally, I think adding the typescript package and running the type checking through the TypeScript compiler with no emit is the best option insteading of using the @typescript-eslint package or letting ESLint do some basic type checks. But I leave that choice up to you.

@simply-alliv simply-alliv changed the title - run npm audit fix for lodash dependency Add TypeScript support Oct 21, 2019
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.

1 participant