asgardeo / asgardeo-auth-react-sdk

To maintain the implementation of Asgardeo React authentication SDK
Apache License 2.0
40 stars 91 forks source link

Update package.json to exclude source code from NPM package #253

Closed s-vamshi closed 1 month ago

s-vamshi commented 1 month ago

Purpose

Describe the problems, issues, or needs driving this feature/fix and include links to related issues in the following format: Resolves issue1, issue2, etc.

Resolves #200

Goals

Describe the solutions that this feature/fix will introduce to resolve the problems

src folder shouldn't be added in package and compiled types should be inside dist/types

Approach

Describe how you are implementing the solutions. Include an animated GIF or screenshot if the change affects the UI (email documentation@wso2.com to review all UI text). Include a link to a Markdown file or Google doc if the feature write-up is too long to paste here.

Added .npmignore file to ignore src and added decelerationDir key in tsconfig.json to put complied types in dist/types/src

s-vamshi commented 1 month ago

Hey @NipuniBhagya @pavinduLakshan done with the changes

s-vamshi commented 1 month ago

@NipuniBhagya can you please review this now made the changes

brionmario commented 1 month ago

Hi @s-vamshi,

While .npmignore seems like a viable option to overcome the issue, i think we can resolve this with a simple files entry in the package.json.

Have you considered that option?

IMO, since we don't go into granular level to which files in include, rather we can just simply include dist, package.json, README, better to go for the files option.

Additionally, we could also copy the LICENSE file at the root to the lib folder, and include that as well in the distribution.

WDYT?

s-vamshi commented 1 month ago

Hey @brionmario, I agree with you, as lib folder didn't have .gitignore I thought of going with .npmignore.

I will add files section in package.json and push them!

brionmario commented 1 month ago

Hey @brionmario, I agree with you, as lib folder didn't have .gitignore I thought of going with .npmignore.

I will add files section in package.json and push them!

Cool 🙌

Lets duplicate the LICENSE inside lib and include that as well :)

s-vamshi commented 1 month ago

Sure 🚀

s-vamshi commented 1 month ago

hey @brionmario, done with changes

s-vamshi commented 1 month ago

Hey @brionmario @NipuniBhagya @pavinduLakshan I have made the changes as suggested can you please review this PR?

s-vamshi commented 1 month ago

Hey @brionmario @NipuniBhagya @pavinduLakshan can you please review this PR?

brionmario commented 1 month ago

LGTM too. Merged!

brionmario commented 1 month ago

🚢 Shipped with v5.2.0

brionmario commented 1 month ago

Thanks @s-vamshi for the contribution 🎉

Looks like the fix is working as expected. If time permits, please try the released version in a React app and do a regression test round!

Screenshot 2024-10-14 at 19 15 26
pavinduLakshan commented 1 month ago

Thanks @s-vamshi for the contribution. This just reduced the package size by 10% (744KB -> 659KB). Great work!

s-vamshi commented 1 month ago

Thanks @s-vamshi for the contribution 🎉

Looks like the fix is working as expected. If time permits, please try the released version in a React app and do a regression test round!

Screenshot 2024-10-14 at 19 15 26

Sure I will do regression testing

NipuniBhagya commented 1 month ago

Hi @s-vamshi,

🎉 Thank you for your contribution! 🎉

We appreciate your effort and the time you've taken to submit this PR. As part of the Hacktoberfest process, please make sure to fill out the form [1] shared to ensure we can properly track your contribution.

[1] - https://docs.google.com/forms/d/1RT5fdrj7DvJCtzHOpvAIbN-4d08RswCOK_GZswZKR8w/viewform?edit_requested=true

Once again, thank you for your support, and we're excited to have you involved in our project! 🙌