Skip to content
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.

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

feat: Convert buffer image to PNG format #17

Merged
merged 3 commits into from
Jan 11, 2024
Merged

Conversation

JEM1224
Copy link
Contributor

@JEM1224 JEM1224 commented Jan 6, 2024

안녕하세요 은재님!
깃에서 이미지를 업로드할 때 저장되는 주소가 변경된거같습니다.

//전
https://user-images.githubusercontent.com/adfadsf.JPG

//후
https://github.com/JEM1224/blog/assets/101504594/dfc2ae84-0793-40ec-9152-984664d21289`

파일이 버퍼로 저장돼서 버퍼 형태를 이미지로 변경하는 코드를 추가했습니다.

  • 이미지 변환을 위한 sharp 패키지를 추가했습니다.
    const validExtensionsRegex = /\.(jpg|jpeg|png)$/i

    if (!validExtensionsRegex.test(image.filename)) {
      await sharp(`./${newImageFilename}`)
        .toFormat('png')
        .toFile(`${newImageFilename}.png`)
      const dirpath = path.join(dirname, newImageFilename)
      newImageFilename += '.png'
      fs.unlinkSync(dirpath)
    }
  • 확장자가 없는 경우 버퍼 파일로 생각해 이미지 변환을 했습니다.
  • newImageFilename에 확장자를 추가했습니다.
  • fs.unlinkSync을 사용하여 원본 파일을 삭제했습니다.

기여를 처음해보는지라 틀린 점이 있다면 말씀해 주세요 ! 유튜브 너무 잘보고있습니다 !! 🙇‍♀️

Copy link
Owner

@eunjae-lee eunjae-lee left a comment

Choose a reason for hiding this comment

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

안녕하세요! pull request 만들어 주셔서 감사합니다 🙂 질문 하나 남겼으니 확인 부탁드려요!

src/main.ts Outdated
fs.writeFileSync(
path.join(dirname, newImageFilename),
await download(image.filename)
)

const validExtensionsRegex = /\.(jpg|jpeg|png)$/i
Copy link
Owner

Choose a reason for hiding this comment

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

"확장자가 없으면.." 이라기보다 "저 셋 중에 하나로 끝나지 않으면" 의 조건으로 작업을 하신 거 같은데, 어떤 게 맞으려나요? 혹시 .tiff 라던가 이런 확장자의 파일들도 다 png 로 변환을 해줘야 하는 걸까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

제가 헷갈리게 썼네요 ㅠㅠ 이미지 확장자만 확인하는 의미로 작성 했습니다..!!

Copy link
Owner

Choose a reason for hiding this comment

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

@JEM1224 아 그러면 저 좀더 궁금한 게, 위의 세 확장자가 아니라 다른 이미지, 예를 들면 .tiff, .gif 이런 것들도 다 아래 if 분기를 타게 돼서 png 로 변환될텐데, 그래도 되는 건지...? 아니면 확장자가 아예 없는 경우에만 아래 분기를 타야 하는건지? 어떤 게 맞을까요?

Downloaded the 'file-type' library to determine the file extension in a buffer-formatted file
src/main.ts Outdated
Comment on lines 131 to 135
}).toFile(`newImageFilename.${imageType.ext}`)
} else {
await sharp(`./${newImageFilename}`).toFile(
`newImageFilename.${imageType.ext}`
)
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
}).toFile(`newImageFilename.${imageType.ext}`)
} else {
await sharp(`./${newImageFilename}`).toFile(
`newImageFilename.${imageType.ext}`
)
}).toFile(`${newImageFilename}.${imageType.ext}`)
} else {
await sharp(`./${newImageFilename}`).toFile(
`${newImageFilename}.${imageType.ext}`
)

이렇게 되는게 맞으려나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eunjae-lee
커밋만 하고 글을 작성하지 못했네요 ㅠㅠ
말씀하신대로 이미지 확장자를 제외한 다른 확장자를 고려하지 못해서 sharp 에서 변환이 가능한 확장자만 변환하도록 했습니다!

  1. 확장자가 존재하지 않는다면
  2. 버퍼 파일을 학인해 파일 종류를 확인합니다.
  3. sharp에서 변환이 가능하다면 변환을 진행합니다.
    3-1. gif 일 때는 옵션을 따로 설정합니다.
    limitInputPixels: false, //이미지 픽셀 설정 제한을 해제 
    animated: true, // animated 설정 
    density: 1 // 해상도설정, 기본값을 했을 때 오래 걸려 낮췄습니다. 

참고

  1. sharp.cache(false) : 파일 캐시가 존재할 때 원본 파일의 삭제를 막아서 캐시 설정을 해제했습니다.
    참고

Copy link
Owner

Choose a reason for hiding this comment

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

설명 감사합니다! 그럼 마지막으로 제가 저 위에 suggestion 남긴 것만 확인 부탁드려요! newImageFilename 변수값이 제대로 사용되지 않은 거 같아서요.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

앗 그러네요 ! 수정하겠습니다!

Copy link
Owner

@eunjae-lee eunjae-lee left a comment

Choose a reason for hiding this comment

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

작업 감사합니다! 😊

@eunjae-lee eunjae-lee merged commit b31eee3 into eunjae-lee:main Jan 11, 2024
@eunjae-lee
Copy link
Owner

@JEM1224 1.2.0 버전으로 배포 됐습니다!

uses: eunjae-lee/issue-to-markdown@v1 으로 사용하고 계셨다면 저절로 새 버전이 적용될 거에요! :)

@JEM1224
Copy link
Contributor Author

JEM1224 commented Jan 11, 2024

감사합니다 !! 잘 쓰겠습니당 ㅎㅎㅎ https://www.jungmin.xyz 블로그도 유튜브 영상보고 만들었어요!

@JEM1224
Copy link
Contributor Author

JEM1224 commented Jan 12, 2024

@eunjae-lee

/home/runner/work/_actions/eunjae-lee/issue-to-markdown/v1/dist/index.js:44469
throw new Error(help.join('\n'));
      ^
Error: Could not load the "sharp" module using the linux-x64 runtime
Possible solutions:
- Ensure optional dependencies can be installed:
  npm install --include=optional sharp
  yarn add sharp --ignore-engines
- Ensure your package manager supports multi-platform installation:
  See https://sharp.pixelplumbing.com/install#cross-platform
- Add platform-specific dependencies:
  npm install --os=linux --cpu=x64 sharp
  npm install --force @img/sharp-linux-x64
- Consult the installation documentation:
  See https://sharp.pixelplumbing.com/install

sharp가 운영체제에 따라 설치되는 모듈이 달라 cross-platform으로 다운받아야 하네요 ㅜ

scripts/depoly.mjs 에 추가해야 하나요?

await $`npm pkg set version=${version}`
await $`yarn all`
await $`npm uninstall sharp` //추가
await $`npm install --platform=linux --arch=x64 sharp` //추가

참고 : https://xn--wo0bx9uoke.com/89

@eunjae-lee
Copy link
Owner

아 🥲 왠지 sharp 에 그런 이슈가 있을 것만 같았는데... 위에 적어주신 걸로는 잠깐 해봤는데 안되네요. 혹시 시간이 되신다면 sharp 대신 다른 라이브러리 (https://github.com/jimp-dev/jimp 라던가)로 교체 작업을 해주실 수 있을까요? 제가 지금은 이 작업을 하기가 어려워서요.

@JEM1224
Copy link
Contributor Author

JEM1224 commented Jan 13, 2024

넵 ! 교체하고 PR올리겠습니다!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants