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: migrate to TypeScript and ESM #1250

Open
DreamOfIce opened this issue May 20, 2023 · 8 comments
Open

feat: migrate to TypeScript and ESM #1250

DreamOfIce opened this issue May 20, 2023 · 8 comments

Comments

@DreamOfIce
Copy link

DreamOfIce commented May 20, 2023

TypeScript

I think it might be possible to make this change in v4, and here are some reasons why:

  • Increase readability and lower the threshold for contribution (it's a bit too difficult to contribute now)
  • Can generate declaration files directly instead of maintaining dts manually

I think this change would reduce your maintenance burden in the long run

ESM

Maybe we can write the code in ESM and then use a tool like esbuild to bundle the esm into cjs and iife while compiling the TypeScript

@DreamOfIce DreamOfIce changed the title feat: migrate to TypeScript feat: migrate to TypeScript and ESM May 20, 2023
@zloirock
Copy link
Owner

Migration to ESM is planned.

Migration to TypeScript is a controversial question. I'm not a big fan of TypeScript. In some internal cases, it could cause some problems. Anyway, I think this needs to be thought about.

However, adding TypeScript definitions is required and planned since the usage of core-js (proposals, pure version, tooling) without it in TypeScript is problematic. Adding it to tooling is already started - #1163, #1235.

@DreamOfIce
Copy link
Author

Considering the positive effects, I think it might be worth it 😄

@DreamOfIce
Copy link
Author

Also, has #1221 been abandoned? You haven't replied for a long time 😓

@zloirock
Copy link
Owner

@DreamOfIce It's not abandoned. Sorry. After some events and burnout, I decided to take a little vacation with my family, so I spent less time than usual on the project. I'll take care of this PR soon.

@DreamOfIce
Copy link
Author

However, adding TypeScript definitions is required and planned since the usage of core-js (proposals, pure version, tooling) without it in TypeScript is problematic.

I have converted all type declarations to typescript in #1221 and checked them against the source code and the official TypeScript declaration file.
Maybe we can write them to a separate file for use.

@jsejcksn
Copy link

@DreamOfIce Perhaps TS types can be provided via JSDoc as a first step instead of attempting to maintain declaration files separately. This will allow for generation of declaration files and also provide types in supported tooling even without that step. Additionally, it will make it easier to refactor to TS in the future (if that's desired) because the types will be colocated in comments with the implementation code.

@DreamOfIce
Copy link
Author

Perhaps TS types can be provided via JSDoc as a first step instead of attempting to maintain declaration files separately.

But we have maintained a type declaration (non-standard TS syntax) for the docs. And converted to standard typescript by me in #1221.

BTW, it is a good idea to add some comments using JSDoc
In fact the new type declaration has some JSDoc comments mixed in, e.g:
https://github.com/DreamOfIce/core-js/blob/afe5b233ed91ee9311be6e397c353bd4de8680ff/docs/features/es-standard/number.md?plain=1#L57-L58

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants
@jsejcksn @zloirock @DreamOfIce and others