[WIP] feat: rewrite project to typescript #1109

aldy505 · 2023-03-22T05:55:13Z

aldy505

A few notes on the progress of this rewrite. It's weekend so I intend to continue this PR later today (or tomorrow).

I have a question about doing a proper integration test, but that might come later.

aldy505 · 2023-03-25T05:24:07Z

.eslintrc.js

@@ -23,19 +42,41 @@ module.exports = {
    ],
    'linebreak-style': [
      'error',
-      (process.platform === 'win32' ? 'windows' : 'unix') // all windows platforms are denoted by win32
+      'unix'


Make the default linebreak style as LF (unix-style), as this ternary configuration really broke on every text editor / IDE I use on WIndows.

aldy505 · 2023-03-25T05:24:52Z

.eslintrc.js

+    'no-console': ['error'],
+    'no-var': ['error'],
+    'no-empty-function': ['error'],
+    'comma-dangle': ['error', 'never'],
+    'prefer-const': ['error'],
+    'quotes': ['error', 'single'],
+    'comma-spacing': ['error', { before: false, after: true }],
+    'semi-spacing': ['warn', { before: false, after: true }],
+    'space-before-blocks': ['warn', 'always'],
+    'switch-colon-spacing': ['warn', { after: true, before: false }],
+    'keyword-spacing': ['warn', { before: true, after: true }],
+    'template-curly-spacing': ['error', 'never'],
+    'rest-spread-spacing': ['error', 'never'],
+    'no-multi-spaces': ['warn', { ignoreEOLComments: false }],


There are applied for consistent code style. The most prominent one is "prefer-const" to avoid any other var on the codebase.

We could use prettier and use the default configuration to enforce it. And configure husky

aldy505 · 2023-03-25T05:27:32Z

package.json

+    "compile": "rollup -c",
+    "test": "mocha test/**/*.js",
+    "lint": "eslint --ignore-path .gitignore .",
+    "lint-fix": "eslint --ignore-path .gitignore --fix .",
+    "prepublishOnly": "npm run test",
+    "functional": "mocha test/functional/*.js ",
+    "browserify": "npm run compile",


Removing gulp, and replacing it with rollup just for the build step. For everything else, we can directly run their CLI commands (mocha, eslint).

Rollup itself isn't a shiny bundler. A shiny bundler would be esbuild, bun or turbo.

aldy505 · 2023-03-25T05:29:10Z

rollup.config.js

+    {
+      file: 'dist/main/minio.cjs',
+      format: 'cjs',
+      exports: 'default'
+    },
+    {
+      file: 'dist/main/minio.mjs',
+      format: 'es'
+    },


Provide both ESM and CommonJS output, as the Javascript ecosystem is actually still on war between CJS vs ESM.

See:

The ESM move sindresorhus/meta#15

https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c

aldy505 · 2023-03-25T05:29:37Z

rollup.config.js

+      file: 'dist/main/minio.iife.js',
+      format: 'iife',
+      name: 'MinIO',
+      plugins: [terser()]


IIFE output is actually optional, as I believe this will be a broken build once used.

aldy505 · 2023-03-25T05:31:46Z

tsconfig.json

+    "declaration": true,                              /* Generate .d.ts files from TypeScript and JavaScript files in your project. */
+    // "declarationMap": true,                           /* Create sourcemaps for d.ts files. */
+    // "emitDeclarationOnly": true,                      /* Only output d.ts files and not JavaScript files. */
+    "sourceMap": true,                                /* Create source map files for emitted JavaScript files. */


I'm actually not sure whether we should emit the source map or not. Probably not though, but I guess it's a good thing if people want to debug the library once there is any problem or errors.

aldy505 · 2023-03-25T05:33:54Z

src/main/internals/lodash.ts

+ * isEmpty({ 'a': 1 })
+ * // => false
+ */
+function isEmpty(value: unknown): boolean {


I'm intending to include vendored libnraries that are used not much throughout the SDK, just for turning down the final dependency size of the SDK itself.

For example if as an SDK, you're required to install Lodash that'll take a few KBs, we can trim that down into a few bytes not with using tree shaking, but with vendoring the needed functions.

aldy505 · 2023-03-25T05:35:10Z

src/main/internals/buffer-crc32.ts

+  return crc ^ -1
+}
+
+function crc32() {


Other than vendoring some libraries that are not used often, I wanted to vendor some libraries that not really maintained anymore and might contain vulnerabilities.

This crc32 library is simple enough that I decided just to do that. And yes, we should inlude a unit test for the specific libraries that we vendored.

aldy505

Another progress update.

If anyone's want to help, you could:

Create typings for the signing.ts file (link)
Create typings for transformers.ts file (link)
Create typings for the xml-parsers.ts file (link)

As I will not touch those files until some of the core function on minio.ts file is completed.

aldy505 · 2023-04-01T10:47:27Z

src/main/minio.ts

+  getObject(bucketName: string, objectName: string, callback: (err: Error | null) => void): void
+  getObject(bucketName: string, objectName: string, getOpts: GetObjectOptions, callback: (err: Error | null) => void): void
+  getObject(bucketName: string, objectName: string, ...rest: [(err: Error | null) => void] | [GetObjectOptions, (err: Error | null) => void]): void {


This will be the style of pattern on how we'll implement function overloading for the sake of backward compatibility. See https://tsplay.dev/wXvnOm for Typescript playground sample.

i will try to update types for the above links. Thank you @aldy505 .

Could you please resolve the conflicts ?

aldy505 · 2023-04-01T10:48:13Z

src/main/minio.ts

@@ -3628,134 +3676,5 @@ Client.prototype.getObjectLegalHold=promisify(Client.prototype.getObjectLegalHol
 Client.prototype.composeObject = promisify(Client.prototype.composeObject)
 Client.prototype.selectObjectContent=promisify(Client.prototype.selectObjectContent)

-export class CopyConditions {


Moved CopyConditions class to a separate file

aldy505 · 2023-04-01T10:48:34Z

src/main/minio.ts

-}
-
-// Build PostPolicy object that can be signed by presignedPostPolicy
-export class PostPolicy {


Also moved PostPolicy to a separate file, we'll re-export them from the minio.ts file later on.

aldy505 · 2023-04-01T10:50:43Z

src/main/minio.ts

+          response.on('data', () => {
+            // do nothing
+          })


See https://eslint.org/docs/latest/rules/no-empty-function

prakashsvmx · 2023-04-04T08:44:47Z

I looked at the PR in detail @aldy505 .
My humble suggestion is, we can take the approach of progressive improvement than one single PR as it is more manageable and easy for migration.

prakashsvmx · 2023-04-04T10:58:31Z

@aldy505 - i would be migrating to ts progressively via https://github.com/minio/minio-js/pull/1114` and ensure every step passes CI test

aldy505 · 2023-04-04T11:01:21Z

@prakashsvmx okay. can you create a list of checkbox that we need to do for converting the project to typescript? I only available to code OSS stuff on weekends, but I can respond to messages or a small portion of code reviews on weekdays.

aldy505 · 2023-04-10T14:58:28Z

I'll leave this PR be for future reference on the Typescript rewrite journey. Please do not close this PR as long as the project is still in Javascript (or the rewrite hasn't been completed yet).

All of us should work on a separate small PRs.

prakashsvmx · 2023-05-05T07:22:42Z

The progressive upgrade PRs have been merged. closing this in favour of them. Thank you for collaborating 👍

aldy505 added 3 commits March 22, 2023 11:37

feat: scaffold typescript configuration

050e9f7

chore: strictier eslint configuration

89a521b

feat: more rewrite tasks

d6dbefb

harshavardhana requested a review from prakashsvmx March 24, 2023 04:59

aldy505 commented Mar 25, 2023

View reviewed changes

feat: more rewrite tasks (2)

fb09d0b

aldy505 commented Apr 1, 2023

View reviewed changes

This was referenced Apr 9, 2023

build: update workflow to support ts #1119

Merged

style: add prettier, lint-staged, husky and eslint rules #1120

Merged

prakashsvmx closed this May 5, 2023

aldy505 deleted the feat/rewrite-typescript branch May 5, 2023 14:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] feat: rewrite project to typescript #1109

[WIP] feat: rewrite project to typescript #1109

aldy505 commented Mar 22, 2023

aldy505 left a comment

aldy505 Mar 25, 2023

aldy505 Mar 25, 2023

prakashsvmx Mar 25, 2023

aldy505 Mar 25, 2023

aldy505 Mar 25, 2023

aldy505 Mar 25, 2023

aldy505 Mar 25, 2023

aldy505 Mar 25, 2023

aldy505 Mar 25, 2023

aldy505 left a comment

aldy505 Apr 1, 2023

prakashsvmx Apr 3, 2023

aldy505 Apr 1, 2023

aldy505 Apr 1, 2023

aldy505 Apr 1, 2023

prakashsvmx commented Apr 4, 2023

prakashsvmx commented Apr 4, 2023

aldy505 commented Apr 4, 2023 •

edited

Loading

aldy505 commented Apr 10, 2023

prakashsvmx commented May 5, 2023

[WIP] feat: rewrite project to typescript #1109

[WIP] feat: rewrite project to typescript #1109

Conversation

aldy505 commented Mar 22, 2023

aldy505 left a 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

aldy505 left a 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

prakashsvmx commented Apr 4, 2023

prakashsvmx commented Apr 4, 2023

aldy505 commented Apr 4, 2023 • edited Loading

aldy505 commented Apr 10, 2023

prakashsvmx commented May 5, 2023

aldy505 commented Apr 4, 2023 •

edited

Loading