add file exists check to prevent override of existing .gitignore file… #1630

maxgaurav · 2022-04-29T10:01:02Z

… within collection

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ X ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

When attempting to create new application through nest cli when collection is provided which contains a gitignore file either fails with git repository init failure or existing gitignore file being overridden by default nestjs template content.
Issue Number: #1630

What is the new behavior?

The create git ignore file will skip creation if the file already exists

Does this PR introduce a breaking change?

[ ] Yes
[ X ] No

Other information

… within collection

micalevisk · 2022-04-29T21:17:51Z

actions/new.action.ts

+  return fileExists(filePath).then(exists => {
+    if (!exists) {
+      return promisify(fs.writeFile)(filePath, fileContent);
+    }
+    return;
+  });


why not use async/await here

Suggested change

return fileExists(filePath).then(exists => {

if (!exists) {

return promisify(fs.writeFile)(filePath, fileContent);

}

return;

});

const gitIgnoreFileExists = await fileExists(filePath);

if (!gitIgnoreFileExists) return promisify(fs.writeFile)(filePath, fileContent);

micalevisk · 2022-04-29T21:18:30Z

actions/new.action.ts

@@ -203,7 +203,12 @@ const initializeGitRepository = async (dir: string) => {
 const createGitIgnoreFile = (dir: string, content?: string) => {


Suggested change

const createGitIgnoreFile = (dir: string, content?: string) => {

const createGitIgnoreFile = async (dir: string, content?: string) => {

Hi sorry for the dealy in response. Got stuck in my main project. I have no problem doing it this way, I was just trying to get the flow what was already there. If needed I can change.

I believe async/await will be better to follow

kamilmysliwiec · 2022-05-18T12:02:18Z

🏓

micalevisk · 2022-05-19T04:18:08Z

actions/new.action.ts

+const fileExists = (path: string) => {
+  return promisify(fs.access)(path).then(() => true).catch((err: any) => {


actually, I don't see any reason to use promises here because all the other tasks will need to wait the createGitIgnoreFile one

So can you replace fs.access by fs.accessSync?

Just changed the flow to use the synchronous access and mainly reverted the createGitIgnoreFile to be mostly the same code except adding the check to file exist then it returns.

can you run npm ci && npm run format to fix formatting issues?

but only commit the actions/new.action.ts xD

nvm, let me help you with that. I just ran that command myself

micalevisk · 2022-05-19T04:36:30Z

actions/new.action.ts

@@ -250,4 +254,17 @@ export const retrieveCols = () => {
  }
 };

+const fileExists = (path: string) => {
+  try{


Suggested change

try{

try {

micalevisk · 2022-05-19T04:37:13Z

actions/new.action.ts

+
+    throw err;
+  }
+}


Suggested change

}

};

On a separate note, this whole gitignore problem is there as the npm does not copy .gitignore files to the distributions. Since we are using angular schematics which allows template prefix/suffix to be added which can be automatically removed through the schematics build process. I have achieved the same for my collection
Reference: https://github.com/maxgaurav/nestjs-template-schematics

Would it be okay that do the same for the nestjs schematics as well by opening a PR which can allow managing of .gitignore file through schematics.

sure! I'll close the PR #1649 then

Which means that we can close this PR as well, right? then after your fix on that schematics issue, we could open another one to drop the workaround implemented on this repo

Yes, makes sense. But form then the responsibility of having .gitingore file will be to the schematics, if not provided then gitingore will not be there in the project structure.

Once I complete the functionality I will map the pull request here for refernce.

add file exists check to prevent override of existing .gitignore file…

d3d6ef7

… within collection

jmcdo29 approved these changes Apr 29, 2022

View reviewed changes

Tony133 approved these changes Apr 29, 2022

View reviewed changes

micalevisk reviewed Apr 29, 2022

View reviewed changes

kamilmysliwiec mentioned this pull request May 4, 2022

New application with collection already containing a .gitignore fails #1628

Closed

4 tasks

micalevisk reviewed May 19, 2022

View reviewed changes

change fileExists to be synchronous

097d53f

maxgaurav force-pushed the master branch from 22b862e to 097d53f Compare May 19, 2022 04:31

micalevisk suggested changes May 19, 2022

View reviewed changes

fix formatting issue in add actions

847192b

micalevisk approved these changes May 19, 2022

View reviewed changes

Tony133 approved these changes May 19, 2022

View reviewed changes

micalevisk mentioned this pull request May 19, 2022

Add .env to generated .gitignore file nestjs/schematics#1004

Closed

1 task

kamilmysliwiec merged commit 64f8334 into nestjs:master Jul 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add file exists check to prevent override of existing .gitignore file… #1630

add file exists check to prevent override of existing .gitignore file… #1630

maxgaurav commented Apr 29, 2022 •

edited

micalevisk Apr 29, 2022

micalevisk Apr 29, 2022

maxgaurav May 19, 2022

micalevisk May 19, 2022

kamilmysliwiec commented May 18, 2022

micalevisk May 19, 2022 •

edited

maxgaurav May 19, 2022

maxgaurav May 19, 2022

micalevisk May 19, 2022

micalevisk May 19, 2022

micalevisk May 19, 2022

micalevisk May 19, 2022

micalevisk May 19, 2022

maxgaurav May 19, 2022

micalevisk May 19, 2022

maxgaurav May 19, 2022

maxgaurav May 19, 2022

		@@ -203,7 +203,12 @@ const initializeGitRepository = async (dir: string) => {
		const createGitIgnoreFile = (dir: string, content?: string) => {

	const createGitIgnoreFile = (dir: string, content?: string) => {
	const createGitIgnoreFile = async (dir: string, content?: string) => {

		const fileExists = (path: string) => {
		return promisify(fs.access)(path).then(() => true).catch((err: any) => {

add file exists check to prevent override of existing .gitignore file… #1630

add file exists check to prevent override of existing .gitignore file… #1630

Conversation

maxgaurav commented Apr 29, 2022 • edited

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kamilmysliwiec commented May 18, 2022

micalevisk May 19, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxgaurav commented Apr 29, 2022 •

edited

micalevisk May 19, 2022 •

edited