StaticMania / keep-react

Keep React is an open-source component library built on Tailwind CSS and React.js. It provides a versatile set of pre-designed UI components to build modern web applications.
https://react.keepdesign.io
MIT License
1.32k stars 115 forks source link

Issue 62/publint suggession and warnings #73

Closed ashutoshbw closed 9 months ago

ashutoshbw commented 10 months ago

Resolved all the warnings and suggestion given by publint. You can check locally by running npx publint.

I've tested my changes in Vite and Nextjs apps and found no issues yet.

I've tried to make the changes in a backward compatible way. The build system can be made a lot more efficient and simpler by moving to ESM only package.

While solving this I also noticed the .tsbuildinfo files were before left unused because the whole lib was deleted first in every new build. I solved it by making another script called build:lib:dev for develop time building. It also uses a script called prepare-lib.js to fix the extension warnings publint was giving. The lib-untouched folder contains files that tsc emits along with the .tsbuildinfo files and the minified css.

For the production build the script name is same as before that is build:lib. It will build from scratch.

closes #62

netlify[bot] commented 10 months ago

Deploy Preview for keep-react-sm ready!

Name Link
Latest commit 05547a5619e66166f77ccd23e3f7dff343d4529f
Latest deploy log https://app.netlify.com/sites/keep-react-sm/deploys/655ccf98d4e84d000942e2bf
Deploy Preview https://deploy-preview-73--keep-react-sm.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

ashutoshbw commented 10 months ago

If this is merged, I've a suggestion for the next major release:

To rename the custom subpath in "exports" to something simpler. For example: "./preset" and "./css".

It will then let us import them more simple and user-friendly way:

import keepPreset from "keep-react/preset";

and

@import "keep-react/css";
Arifulislam5577 commented 10 months ago

Dear @ashutoshbw,

Your effort is very important to us. This reduced our library file site by almost half. Publint's warnings are gone but some issues still remain. If you look again it might be fixed too.

image

ashutoshbw commented 10 months ago

Dear @Arifulislam5577, thanks for testing it out and having trust in me. I've not seen such error screen before. Can you please tell me the steps that you followed to get here?

Before there was no --declaration flag to build:lib:cjs, but in my changes I've added one. So the library file size should increase a bit!

Are you testing it from a Windows machine? I'm guessing the problem is due to the fact that I have not made prepare-lib.js cross-platform compatible. I will fix it soon.

ashutoshbw commented 10 months ago

I had hard coded the path separator. It was probably causing the issue. I've fixed it. I hope it will work fine now. Please test it out and let me know if the issues still exist. And also please tell me how to get to that error screen.

Arifulislam5577 commented 10 months ago

Hello @ashutoshbw,

Fantastic news! Your recent contributions have made a significant impact on our library, reducing the file size to an impressive 389kb and eliminating those pesky publint.dev warnings. Your attention to detail in importing presets and CSS files has added a touch of elegance to our codebase, and we truly appreciate it.

While everything is now working smoothly, we've identified a couple of type-related issues in the datepicker and slider components. Not to worry, though – we've got it on our radar, and I'll be addressing these hiccups shortly. Once resolved, these improvements will seamlessly integrate into our next release.

Your commitment to enhancing Keep React has not gone unnoticed, and we're excited about the positive changes you've brought to the table. We eagerly anticipate your future contributions and the continued success of our collaborative efforts.

Best regards, Md Ariful Islam

saiful7778 commented 10 months ago

Sorry to say, I didn't see any error. Can you clarify it for better understanding?

On Fri, 24 Nov 2023 at 13:01, Md Ariful Islam @.***> wrote:

Hello @ashutoshbw https://github.com/ashutoshbw,

Fantastic news! Your recent contributions have made a significant impact on our library, reducing the file size to an impressive 389kb and eliminating those pesky publint.dev warnings. Your attention to detail in importing presets and CSS files has added a touch of elegance to our codebase, and we truly appreciate it.

While everything is now working smoothly, we've identified a couple of type-related issues in the datepicker and slider components. Not to worry, though – we've got it on our radar, and I'll be addressing these hiccups shortly. Once resolved, these improvements will seamlessly integrate into our next release.

Your commitment to enhancing Keep React has not gone unnoticed, and we're excited about the positive changes you've brought to the table. We eagerly anticipate your future contributions and the continued success of our collaborative efforts.

Best regards, Md Ariful Islam

— Reply to this email directly, view it on GitHub https://github.com/StaticMania/keep-react/pull/73#issuecomment-1825227027, or unsubscribe https://github.com/notifications/unsubscribe-auth/AT3YJ5LC7IOBXMK3B7R2TRLYGBA3VAVCNFSM6AAAAAA7RY6UGKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRVGIZDOMBSG4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

ashutoshbw commented 10 months ago

Hi @Arifulislam5577 , It's great to hear that the changes have been successful. I'm definitely eager to contribute more. Thank you!

vercel[bot] commented 9 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
keep-react ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 13, 2023 1:25am
Arifulislam5577 commented 9 months ago

Hi @ashutoshbw,

I hope you're doing well. I've tried incorporating your changes, but unfortunately, I encountered two errors in the Slider and DatePicker components. Despite my efforts, I couldn't resolve these issues.

To help us move forward, could you please install the keep-react-demo package and demonstrate the errors by importing the Slider and DatePicker components? Your assistance in resolving this matter would be greatly appreciated. Also you can checkout this repo for more info : https://github.com/Arifulislam5577/keep-test

Looking forward to your prompt response.

Best regards, Md Ariful Islam

ashutoshbw commented 9 months ago

Hi @Arifulislam5577 Thank you! I hope you are well too. I've tested the keep-react-demo package and the repository you provided. I also encountered some errors. I am not understanding why it is happening. I'm not sure if I'll be able to solve these issues soon. If I able to solve these issues, I'll let you know.

Arifulislam5577 commented 9 months ago

Hi @ashutoshbw,

Good news! The issue has been resolved, although we had to revert to using the .js extension instead of .cjs. Unfortunately, the React Date Picker library does not yet support the .cjs extension.

If you have any alternative suggestions or ways to achieve the same outcome with a different approach, I'm open to exploring them. Your input and expertise are greatly appreciated.

Looking forward to your guidance on this matter.

Best regards, @Arifulislam5577

ashutoshbw commented 9 months ago

Hi @Arifulislam5577,

I'm pleased to hear that things are improving. I value your open-mindedness and your interest in considering my suggestion.

If you are interested to develop this library in accordance with contemporary standards, I suggest considering the possibility of rewriting or updating (whether through forking or creating something new) any dependencies that currently do not adhere to modern standards. This proactive step will contribute to ensuring alignment with the latest industry practices.

Arifulislam5577 commented 9 months ago

Hi Dear @ashutoshbw,

Thanks for your awesome suggestion. I think we are on the same page because I already plan to update the two packages that do not currently support the .cjs standard. After making those changes, I'll revisit your suggestion, which will allow us to align with the .cjs standard.

Appreciate your input!

Best, @Arifulislam5577