Skip to content

Ensure the project readme is deployed to NPM also #523

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.

Sign up for GitHub

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

Merged
jamesrweb merged 1 commit into from
Aug 19, 2025
Merged

Ensure the project readme is deployed to NPM also #523

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"homepage": "https://github.com/P5-wrapper/react",
"license": "MIT",
"files": [
"README.md",
"dist/component/*"
],
"types": "./dist/component/main.d.ts",
Expand All @@ -28,6 +29,8 @@
"integrate": "pnpm format:check && pnpm lint && pnpm test && pnpm build",
"lint": "eslint --config config/eslint/eslint.config.ts",
"lint:fix": "pnpm lint --fix",
"prepublishOnly": "cp .github/README.md ./README.md",
Copy link
Contributor

@yevdyko yevdyko Aug 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious, why such a complication and why not put it in the root of the repository as usual? I think the files inside the ".github" directory only relate to the Github configuration and README is needed for documentation regardless of the provider. What don't I know?

Copy link
Member Author

@jamesrweb jamesrweb Aug 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same discussion we had about the configs, I just prefer as much to not be in the root directory as possible and .github is one of three places it can't be kept, so it's grouped with other related documents. Besides, the pre and post setup is standard to NPM itself and so works well for the use case anyway since it's only an issue for NPM deployments anyway.

Copy link
Contributor

@yevdyko yevdyko Aug 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this bug highlights that the current solution might be more of a workaround than an improvement and now we’re considering adding an extra hack on top.

According to the GitHub style guide, the README “should be located in the top-level directory for your product or library’s actual codebase”:

https://google.github.io/styleguide/docguide/READMEs.html#what-to-put-in-your-readme

If we move it to the root, contributors and visitors can immediately understand the project without digging into subfolders.

We could technically add a copy command in a prepublishOnly script, but that only runs right before publishing. It wouldn’t help with local development or repo clarity, and it adds unnecessary complexity. Keeping the README in the root is simpler, avoids extra steps and follows convention. Why not keep it simple? 😀

Copy link
Member Author

@jamesrweb jamesrweb Aug 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The README is read out of the GitHub directory for the repo and temporarily copied for the NPM deployment. I honestly don't see an issue 😅. I do see your point that the two extra scripts are not needed but I don't think it's an issue to have them and it keeps the root cleaner to me. In saying that, if you insist on it being in the root for conventions sake... maybe. I'm still not convinced but if you insist I suppose I can live with that argument but I still prefer it tidied into the GitHub directory personally since it's a supported location and it's only NPM that needs the deploy scripts which have 0 overhead. Let me know what your opinion is and we can see what to do 🤷🏻‍♂️.

"postpublish": "rm ./README.md",
"prettier": "prettier --config config/prettier/prettier.json --ignore-path .gitignore",
"test": "vitest run --silent --config config/vite/vite.config.ts",
"test:coverage": "vitest run --silent --coverage --config config/vite/vite.config.ts",
Expand Down
Loading