Convert to Typescript #39

nicu-chiciuc · 2023-01-11T14:28:06Z

The main goal of this PR is to update all its dependencies (there are several high-severity dependencies at the moment) and to use Typescript so that there are well maintained type definitions.

Steps that have been done

Upgrade to @babel 7

I've updated all the packages to the latest versions.

I've had to update the .babelrc to the latest version by using the @babel/preset-env preset.

Replacing `standard`

standard cannot parse Typescript code, I've tried to use ts-standard instead, but a big issue seems to be that the fixes that it applies can break the code.

For example this:

  const encodedBytesCount = (str.match(/=[\da-fA-F]{2}/g) || []).length

was transformed to this:

  const encodedBytesCount = ((str.match(/=[\da-fA-F]{2}/g) != null) || []).length

I opted to use prettier, and I also changed some of its options so that there are minimal changes to how the code looks.

Completely replacing `standard` with `eslint`

I've adopted typescript-eslint since it's better supported.
I've tried using the eslint-config-standard-with-typescript config, so that we have the same rules as before, but that package required too many peer dependencies, so I decided to rely on the default typescript-eslint rules.

Renaming test files from `-unit.js` to `.unit.js`

For some reason the babel --ignore option didn't want to work when the delimiter is -.
The simplest solution was to replace - with ., and the pattern matcher works.

Fix test `'should Fold flowed text'`

The call

    expect(outputStr).to.equal(foldLines(inputStr, 76, true))

was changed to

    expect(outputStr).to.equal(foldLines(inputStr, true))

since the foldLines function doesn't accept a number (a constant value is used internally).

Emitting the type declarations ".d.ts"

Relying on babel for this doesn't seem to be an option.
I've added another tsconfig.types.json file and a npm run build.types script to specifically generate the type declarations.

The inline source maps

I'm not sure if they bring value, since they are created after the typescript transformation step.

I think for the future it would make more sense to rely on something like https://tsdx.io/ so we don't have to maintain all the boilerplate.

Testing steps

The existing tests should pass npm run test
I've imported this package using npm link in a different project and checked if it still worked

nicu-chiciuc · 2023-01-13T13:39:32Z

.gitignore

 npm-debug.log
 .DS_Store
-package-lock.json


I think it's recommended to have the package-lock inside the source code

nicu-chiciuc · 2023-01-13T13:39:53Z

.prettierrc

@@ -0,0 +1,6 @@
+{
+  "semi": false,


I based these on the way standard was formatting the code

nicu-chiciuc · 2023-01-13T13:40:10Z

.vscode/launch.json

-      "runtimeArgs": [
-          "--nolazy"
-      ],
+      "args": ["./src/${fileBasename}", "--require", "@babel/register", "--reporter", "spec", "--no-timeouts"],


will run just the currently selected test file

nicu-chiciuc · 2023-01-13T13:40:33Z

@types/emailjs-base64.d.ts

@@ -0,0 +1,5 @@
+declare module 'emailjs-base64' {
+  export function encode(input: string | Uint8Array): string


since emailjs-base64 doesn't yet have type declaration, I've declared them temporarily here

nicu-chiciuc · 2023-01-13T13:41:21Z

package.json

@@ -16,38 +17,36 @@
  ],
  "author": "Andris Reinman <[email protected]>",
  "scripts": {
-    "build": "./scripts/build.sh",
-    "lint": "$(npm bin)/standard",
+    "build": "rm -rf ./dist/ && npm run build.types && npm run babel.compile",


the ./scripts/build.sh did too much, automatically creating a commit doesn't seem easy to work with

nicu-chiciuc · 2023-01-13T13:42:03Z

src/charset.ts

  const CHUNK_SZ = 0x8000
  const strs = []

  for (let i = 0; i < arr.length; i += CHUNK_SZ) {
-    strs.push(String.fromCharCode.apply(null, arr.subarray(i, i + CHUNK_SZ)))
+    strs.push(String.fromCharCode.apply(null, Array.from(arr.subarray(i, i + CHUNK_SZ))))


This is one of the places I've had to change the code and add Array.from()

nicu-chiciuc · 2023-01-13T13:43:52Z

src/charset.unit.ts

 import { encode, decode, convert } from './charset'

 describe('#charset', function () {
  describe('#encode', function () {
    it('should encode UTF-8 to ArrayBuffer', function () {
      const str = '신'
-      const encoded = new Uint8Array([0xEC, 0x8B, 0xA0])
+      const encoded = new Uint8Array([0xec, 0x8b, 0xa0])


prettier automatically lower-cases the hex codes. There doesn't seem to be a prettier option around this.
An option would be to use prettier-eslint but that comes with other issues (mostly editor integration) and speed of formatting

prettier/prettier#5158 (comment)

nicu-chiciuc · 2023-01-13T13:46:42Z

src/mimecodec.ts

    }
  }

+  // convert single value arrays to single values
+  const headersObj = Object.fromEntries(


It was hard to work around TS requirements, and it was simpler to do another pass

nicu-chiciuc · 2023-01-13T13:47:39Z

src/mimecodec.ts

  }

  // handle parameter value continuations
  // https://tools.ietf.org/html/rfc2231#section-3

+  const processedParams: Record<


This part had the most actual code changes. Instead of working in the same object, I've split each step to create a new object, so that it can be statically analyzed by TS

nicu-chiciuc · 2023-01-13T13:48:23Z

src/mimecodec.ts

      }
    }
  })

-  return response
+  const responseParams = { ...initialParams, ...concatenatedParams }


and here I put both values in the same object to get the same result

nicu-chiciuc · 2023-01-13T13:54:57Z

@felixhammerl I can't assign someone as a reviewer.

I managed to update the package, while maintain as much of the original source code as possible.

2 things remain to be done:

Choosing a good version number
Figuring out the deployment strategy for NPM

We could rely on Github Actions (instead of Travis) for adding the CI and publishing pipeline, but first we'd need to decide if the direction I was going with this PR is correct.

nicu-chiciuc added 20 commits January 10, 2023 18:50

update babel-cli to @babel/cli

b8ebe4e

run standard --fix over all src files

c1cd5bc

replace babel-register with @babel/register; update babelrc.

5ce7b79

wip add typescript

2736989

rename *-unit.js to *.unit.js so it can be ignored by babel

caa3925

Updating dist files

7e6631f

fix mocha test command

a1e94b0

Updating dist files

73efb1b

update files to Typescript, and use ts-mocha

cf0614b

replace standard with ts-standard

0200ee6

use prettier instead of standard for formatting

fb9776c

add basic types to the functions

72c7851

Remove ramda as a dependecy

c6b5301

Replace standard with typescript-eslint and fix problems

d1ce0ac

Add extensions option to babel cli and generate dist

0e330d8

wip: first refactoring of parseHeaderValue

acaa4ee

fix TS issues in mimecodec.ts

79bb712

remove the number 76 from the test

901a1e2

fix all remaining TS errors

789c313

create the type declaration in a separate step

1844739

nicu-chiciuc commented Jan 13, 2023

View reviewed changes

.gitignore

npm-debug.log

.DS_Store

package-lock.json

Copy link

Author

nicu-chiciuc Jan 13, 2023

Choose a reason for hiding this comment

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

I think it's recommended to have the package-lock inside the source code

nicu-chiciuc commented Jan 13, 2023

View reviewed changes

.prettierrc

@@ -0,0 +1,6 @@

{

"semi": false,

Copy link

Author

nicu-chiciuc Jan 13, 2023

Choose a reason for hiding this comment

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

I based these on the way standard was formatting the code

nicu-chiciuc commented Jan 13, 2023

View reviewed changes

nicu-chiciuc marked this pull request as ready for review January 13, 2023 13:50

nicu-chiciuc mentioned this pull request Feb 28, 2023

[emailjs-mime-codec] Add initial type definitions DefinitelyTyped/DefinitelyTyped#64527

Merged

17 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert to Typescript #39

Convert to Typescript #39

nicu-chiciuc commented Jan 11, 2023 •

edited

Loading

nicu-chiciuc Jan 13, 2023

nicu-chiciuc Jan 13, 2023

nicu-chiciuc Jan 13, 2023

nicu-chiciuc Jan 13, 2023

nicu-chiciuc Jan 13, 2023

nicu-chiciuc Jan 13, 2023

nicu-chiciuc Jan 13, 2023

nicu-chiciuc Jan 13, 2023

nicu-chiciuc Jan 13, 2023

nicu-chiciuc Jan 13, 2023

nicu-chiciuc Jan 13, 2023

nicu-chiciuc commented Jan 13, 2023

		@@ -0,0 +1,5 @@
		declare module 'emailjs-base64' {
		export function encode(input: string \| Uint8Array): string

Convert to Typescript #39

Are you sure you want to change the base?

Convert to Typescript #39

Conversation

nicu-chiciuc commented Jan 11, 2023 • edited Loading

Steps that have been done

Upgrade to @babel 7

Replacing standard

Completely replacing standard with eslint

Renaming test files from *-unit.js to *.unit.js

Fix test 'should Fold flowed text'

Emitting the type declarations ".d.ts"

The inline source maps

Testing steps

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicu-chiciuc commented Jan 13, 2023

nicu-chiciuc commented Jan 11, 2023 •

edited

Loading

Replacing `standard`

Completely replacing `standard` with `eslint`

Renaming test files from `-unit.js` to `.unit.js`

Fix test `'should Fold flowed text'`