search

orling / grapheme-splitter

A JavaScript library that breaks strings into their individual user-perceived characters.
MIT License
926 stars 45 forks source link

Refactor plan #23

Open JLHwung opened 5 years ago

JLHwung commented 5 years ago

@orling

For my personal interest on Unicode, I would like to do a refactor of this library, here is some thoughts come to me:

I would like to work on some of these tips and push it to https://github.com/JLHwung/grapheme-splitter/tree/next

If you are open to changes to this library, I am happy to raise a PR once I finish the refactor.

orling commented 5 years ago

Hi Huang,

Thank you very much for your proposal and eagerness to help!

The library does need an upgrade to Unicode 11 and you are more than welcome to submit a pull request with that.

As for you suggestion to migrate to TypeScript, while it sounds reasonable, I don't think it's a good idea in the long run.

I try to keep this library vanilla JavaScript, without any dependencies and complicated toolchains. I am the only maintainer that is guaranteed to be around to update the library when eg a new Unicode standard comes out. I also have never used TypeScript and generally don't keep up date with JavaScript's framework-of-the-week (not doing JS on a daily basis any more). Migrating to TypeScript will make future upgrades too difficult. A lint and performance check would also be an overkill for a library that is supposed to be deliberately simple.

If you are really into TypeScript, my suggestion is to fork/copy the repo into a parallel library. Just like what this library did with the CoffeeScript grapheme-breaker a long time ago. Just because I have a different vision for it doesn't mean that people wouldn't rather use a TS library if it better fits their needs/toolchains/tastes.

On Fri, Dec 7, 2018 at 05:24 Huáng Jùnliàng notifications@github.com wrote:

@orling https://github.com/orling

For my personal interest on Unicode, I would like to do a refactor of this library, here is some thoughts come to me:

-

Transcribe the whole library into TypeScript so that we do not have to maintain the index.d.ts and index.js. Before we publish any new version, the TypeScript compiler will compile into JavaScript and generate proper type declaration.

Update the library to Unicode 11.0 and draft another branch to Unicode 12.0 beta

Write benchmark of the library and tune performance if needed

Add ESLint + Prettier to maintain good code quality

I would like to work on some of these tips and push it to https://github.com/JLHwung/grapheme-splitter/tree/next

If you are open to changes to this library, I am happy to raise a PR once I finish the refactor.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/orling/grapheme-splitter/issues/23, or mute the thread https://github.com/notifications/unsubscribe-auth/ADpbEPbxfXLwLpakYJHNx94Ro5ZX1sO3ks5u2e1jgaJpZM4ZHxH_ .

JLHwung commented 5 years ago

Hi Orlin

Thanks for a quick reply.

I am the only maintainer that is guaranteed to be around to update the library when eg a new Unicode standard comes out.

Then I would like to help because as an OSS author I can understand the long-term cost of maintaing. An OSS on a single maintainer is risky.

I work on (Java|Type)Script on a daily basis so the toolchain is natural to me. Actually the TypeScript transportation has been finished at my branch.

I do understand your concerns, especially when it comes to the case of CoffeeScript, which was popular in old days but faded out later. As there is not much type annotations on the library, I can resort to a ES2017 version of the library (for better maintainability) and use babel to transpile the code to suite different browser/node requirements. What do you think?

Personally I would not like to copy the library to a TypeScript version because inconsistency might happen during any updating cycle. It is confusing when one is targeting Unicode 12 while the other is still on Unicode 11.

As for the ESLint/Prettier, they are code quality tools and I think they will offer more help than the maintaining cost.

Anyway it might sounds unreasonable for such a drastic change, I can raise PR step by step.

Regards, Huáng Jùnliàng

On Dec 7, 2018, at 3:25 PM, Orlin Georgiev notifications@github.com wrote:

Hi Huang,

Thank you very much for your proposal and eagerness to help!

The library does need an upgrade to Unicode 11 and you are more than welcome to submit a pull request with that.

As for you suggestion to migrate to TypeScript, while it sounds reasonable, I don't think it's a good idea in the long run.

I try to keep this library vanilla JavaScript, without any dependencies and complicated toolchains. I am the only maintainer that is guaranteed to be around to update the library when eg a new Unicode standard comes out. I also have never used TypeScript and generally don't keep up date with JavaScript's framework-of-the-week (not doing JS on a daily basis any more). Migrating to TypeScript will make future upgrades too difficult. A lint and performance check would also be an overkill for a library that is supposed to be deliberately simple.

If you are really into TypeScript, my suggestion is to fork/copy the repo into a parallel library. Just like what this library did with the CoffeeScript grapheme-breaker a long time ago. Just because I have a different vision for it doesn't mean that people wouldn't rather use a TS library if it better fits their needs/toolchains/tastes.

On Fri, Dec 7, 2018 at 05:24 Huáng Jùnliàng notifications@github.com wrote:

@orling https://github.com/orling

For my personal interest on Unicode, I would like to do a refactor of this library, here is some thoughts come to me:

-

Transcribe the whole library into TypeScript so that we do not have to maintain the index.d.ts and index.js. Before we publish any new version, the TypeScript compiler will compile into JavaScript and generate proper type declaration.

Update the library to Unicode 11.0 and draft another branch to Unicode 12.0 beta

Write benchmark of the library and tune performance if needed

Add ESLint + Prettier to maintain good code quality

I would like to work on some of these tips and push it to https://github.com/JLHwung/grapheme-splitter/tree/next

If you are open to changes to this library, I am happy to raise a PR once I finish the refactor.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/orling/grapheme-splitter/issues/23, or mute the thread https://github.com/notifications/unsubscribe-auth/ADpbEPbxfXLwLpakYJHNx94Ro5ZX1sO3ks5u2e1jgaJpZM4ZHxH_ .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/orling/grapheme-splitter/issues/23#issuecomment-445146480, or mute the thread https://github.com/notifications/unsubscribe-auth/ADcNdt0i2Zdt_rGMFkpz0sBCAaT6WR-Pks5u2hgAgaJpZM4ZHxH_.

orling commented 5 years ago

So if I understand correctly, you consider upgrading the code to ES2017 and use babel for backward porting? If so, please update the README with instructions on how to install and run babel. Also the backported version should be in the repo.

An extra lint check may be OK, but again please update the README with instructions how to install and run it. Building and publishing should be possible without it (hopefully the linter will still be around and well supported years from now, but let's not depend on that)

Let me know what you think

On Fri, Dec 7, 2018 at 08:49 Huáng Jùnliàng notifications@github.com wrote:

Hi Orlin

Thanks for a quick reply.

I am the only maintainer that is guaranteed to be around to update the library when eg a new Unicode standard comes out.

Then I would like to help because as an OSS author I can understand the long-term cost of maintaing. An OSS on a single maintainer is risky.

I work on (Java|Type)Script on a daily basis so the toolchain is natural to me. Actually the TypeScript transportation has been finished at my branch.

I do understand your concerns, especially when it comes to the case of CoffeeScript, which was popular in old days but faded out later. As there is not much type annotations on the library, I can resort to a ES2017 version of the library (for better maintainability) and use babel to transpile the code to suite different browser/node requirements. What do you think?

Personally I would not like to copy the library to a TypeScript version because inconsistency might happen during any updating cycle. It is confusing when one is targeting Unicode 12 while the other is still on Unicode 11.

As for the ESLint/Prettier, they are code quality tools and I think they will offer more help than the maintaining cost.

Anyway it might sounds unreasonable for such a drastic change, I can raise PR step by step.

Regards, Huáng Jùnliàng

On Dec 7, 2018, at 3:25 PM, Orlin Georgiev notifications@github.com wrote:

Hi Huang,

Thank you very much for your proposal and eagerness to help!

The library does need an upgrade to Unicode 11 and you are more than welcome to submit a pull request with that.

As for you suggestion to migrate to TypeScript, while it sounds reasonable, I don't think it's a good idea in the long run.

I try to keep this library vanilla JavaScript, without any dependencies and complicated toolchains. I am the only maintainer that is guaranteed to be around to update the library when eg a new Unicode standard comes out. I also have never used TypeScript and generally don't keep up date with JavaScript's framework-of-the-week (not doing JS on a daily basis any more). Migrating to TypeScript will make future upgrades too difficult. A lint and performance check would also be an overkill for a library that is supposed to be deliberately simple.

If you are really into TypeScript, my suggestion is to fork/copy the repo into a parallel library. Just like what this library did with the CoffeeScript grapheme-breaker a long time ago. Just because I have a different vision for it doesn't mean that people wouldn't rather use a TS library if it better fits their needs/toolchains/tastes.

On Fri, Dec 7, 2018 at 05:24 Huáng Jùnliàng notifications@github.com wrote:

@orling https://github.com/orling

For my personal interest on Unicode, I would like to do a refactor of this library, here is some thoughts come to me:

-

Transcribe the whole library into TypeScript so that we do not have to maintain the index.d.ts and index.js. Before we publish any new version, the TypeScript compiler will compile into JavaScript and generate proper type declaration.

Update the library to Unicode 11.0 and draft another branch to Unicode 12.0 beta

Write benchmark of the library and tune performance if needed

Add ESLint + Prettier to maintain good code quality

I would like to work on some of these tips and push it to https://github.com/JLHwung/grapheme-splitter/tree/next

If you are open to changes to this library, I am happy to raise a PR once I finish the refactor.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/orling/grapheme-splitter/issues/23, or mute the thread < https://github.com/notifications/unsubscribe-auth/ADpbEPbxfXLwLpakYJHNx94Ro5ZX1sO3ks5u2e1jgaJpZM4ZHxH_

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub < https://github.com/orling/grapheme-splitter/issues/23#issuecomment-445146480>, or mute the thread < https://github.com/notifications/unsubscribe-auth/ADcNdt0i2Zdt_rGMFkpz0sBCAaT6WR-Pks5u2hgAgaJpZM4ZHxH_ .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/orling/grapheme-splitter/issues/23#issuecomment-445150888, or mute the thread https://github.com/notifications/unsubscribe-auth/ADpbEEXMXKKNknY8vqc6BEhYli6Z5Npqks5u2h2UgaJpZM4ZHxH_ .

JLHwung commented 5 years ago

Hi orling,

I raised #24 to show the current progress on the next branch.

If so, please update the README with instructions on how to install and run babel.

Yes, I drafts a CONTRIBUTING.md to list things developer should do during developments. The reason I prefer CONTRBUTING.md instead of README.md is that the README.md will be shipped to npm package while the latter does not. As an end user, they should not care about whether we are using babel or not. For the optimal npm package size, I document the process in the CONTRIBUTING.md only. The process should be almost transparent for developers.

In the high level, we are sting using

npm install => npm test => npm publish

process, the babel will do the right thing under the hood.

An extra lint check may be OK, but again please update the README with instructions how to install and run it.

The linter will be also installed during npm install step and will live under the precommit git hook so it could lint the code literally every time before you commit a change. However I plan to add linters after we ship the Unicode 11 implementation in case the PR becomes a mammoth epic. 😄

If we agree on the refactor plan, I think this issue can be closed and we can focus on #24.