Skip to content

code-style을 pite로 빌드합니다#96

Merged
keemhyunseok merged 6 commits intomainfrom
repo/94_pite
Mar 11, 2025
Merged

code-style을 pite로 빌드합니다#96
keemhyunseok merged 6 commits intomainfrom
repo/94_pite

Conversation

@keemhyunseok
Copy link
Contributor

Related Issue

Describe your changes

  • 기존 rollup으로 빌드중인 eslint-config, eslint-plugin을 pite로 빌드합니다
  • rollup설정을 제거합니다
  • type:module을 제거합니다. default를 cjs로 바라보게
    • type:module인데 exports에 .mjs 확장자를 사용중
    • 기본 .js를 cjs로 바라보게하고 esm인 경우에 .mjs를 바라도록 하게 변경
  • pite는 기본적으로 cjs, esm모두 preserve하게 빌드됩니다

@keemhyunseok keemhyunseok self-assigned this Feb 28, 2025
@keemhyunseok keemhyunseok requested a review from a team as a code owner February 28, 2025 07:17
@npayfebot
Copy link
Contributor

npayfebot commented Feb 28, 2025

✅ Changeset detected

Latest commit: eec954e

@naverpay/eslint-config, @naverpay/eslint-plugin packages have detected changes.

If no version change is needed, please add skip-detect-change to the label.

The changes in this PR will be included in the next version bump.

powered by: naverpay changeset detect-add actions

export default createViteConfig({
cwd: '.',
entry: ['./lib/index.js'],
ignoredPolyfills: ['esnext.json.parse', 'es.array.push'],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

요것두 폴리필 불필요할 듯 하여 ignore처리했는데 혹시 최신 스펙 사용을 의도한 것이라면 말씀 부탁드립니닷,,

Copy link
Member

Choose a reason for hiding this comment

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

저거 두개 다 아마 ignore 처리해도 될 거에요

문서는 못찾겠는데 사소한 이슈로 알고 있습니다

Copy link
Contributor Author

Choose a reason for hiding this comment

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

일단 코드로 보기에는 진짜 단순 parse, push만 쓴느 것 같아서 ignore할게용

import builtins from 'builtin-modules'

export default createViteConfig({
cwd: '.',
Copy link
Member

Choose a reason for hiding this comment

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

얘 기본값 없었는지 기억이...ㅋㅋㅋ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오 기본값 있긴 하네요

options: {
minify: false,
rollupOptions: {
external: [...builtins],
Copy link
Member

Choose a reason for hiding this comment

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

근데 이거 builtin 기본적으로 빠지게 설정 안했었나.. 기억이 안나네요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

저도 그런줄 알았는데 deps, peerDeps만 기본 external 처리되어 있습니다

Copy link
Contributor

Choose a reason for hiding this comment

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

기본값으로 이거 빼는게 맞을거같네용..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

다음버전에 반영하겠습니닷

@yceffort-naver
Copy link
Contributor

packageJson.json.mjs 막 이런게 생겼었는데 지금은 괜찮죠?

@keemhyunseok
Copy link
Contributor Author

image

이거 생성되는데.. 제외처리 해야하나요?

@2-one-week
Copy link
Member

근데 웨 생성되는거죠?

@keemhyunseok
Copy link
Contributor Author

배럴파일에 package.json export가 있어서 그런 것 같습니다.
glob패턴으로 .json을 제외하면 괜찮을 것 같은데 저거 제외가 필요한 것인가요?

@2-one-week
Copy link
Member

glob패턴으로 .json을 제외하면 괜찮을 것 같은데 저거 제외가 필요한 것인가요?

음... 제 생각엔 .mjs로 빌드는 필요 없을 것 같긴한데, .json export는 필요하지 않을까 싶기도 하고... 있어도 상관 없을 것 같기도 하고..ㅋㅋ

@yceffort-naver
Copy link
Contributor

yceffort-naver commented Mar 4, 2025

package.json 을 포함한 json 은 제외하는게 맞을 것 같아요. 개발자 입장에서는 json 을 읽는게 자연스러운 흐름일것 같아서요..!

@2-one-week
Copy link
Member

package.json 을 포함한 json 은 제외하는게 맞을 것 같아요. 개발자 입장에서는 json 을 읽는게 자연스러운 흐름일것 같아서요..!

근데 이건 json을 package.json dist에 포함되었나..? 혹은 같은 계층에 있는게 맞는가가 확정되어야 괜찮은거 아닐까요??

https://github.com/NaverPayDev/code-style/blob/main/packages/eslint-config/index.js#L9-L12

이 파일 같은 경우는 import 하는 위치가 잘 되어 있어서 괜찮은 것 같은데, 만약 개발자가 실수한다면 json import 를 제대로 못하는 상황이 생길 수 도.. 있지 않나 해서요!

@yceffort-naver
Copy link
Contributor

package.json은 무조건 포함되는 파일이라 괜찮을텐데 (없으면 배포가 안될테니) 나머지 json 은 파일 위치정도는 확인해야 한다는 말씀이실까요?

@2-one-week
Copy link
Member

@keemhyunseok

이거 package.json.mjs 파일 내용 뭔지 알 수 있을까요?

@keemhyunseok
Copy link
Contributor Author

keemhyunseok commented Mar 4, 2025

json의 js화 코드 입니다..!

const name = "@naverpay/eslint-config";
const version = "2.2.2";
const description = "eslint config for naverpay";
const main = "./dist/cjs/index.js";
const module = "./dist/esm/index.mjs";
const exports = {
  ".": {
    "import": "./dist/esm/index.mjs",
    require: "./dist/cjs/index.js",
    "default": "./dist/cjs/index.js"
  },
  "./package.json": "./package.json"
};
const sideEffects = false;
const files = [
  "dist"
];
const scripts = {
  clean: "rm -rf dist",
  prebuild: "pnpm run clean",
  build: "vite build",
  test: "vitest run",
  "test:watch": "vitest watch"
};
const author = "@NaverPayDev/frontend";
const repository = {
  type: "git",
  url: "https://github.com/NaverPayDev/code-style.git",
  directory: "packages/eslint-config"
};
const homepage = "https://github.com/NaverPayDev/code-style";
const keywords = [
  "naverpay",
  "eslint",
  "eslintconfig",
  "eslint-config"
];
const license = "MIT";
const dependencies = {
  "@eslint/eslintrc": "^3.2.0",
  "@eslint/js": "^9.15.0",
  "@naverpay/eslint-plugin": "workspace:*",
  "eslint-config-prettier": "^9.1.0",
  "eslint-plugin-import": ">=2.31.0",
  "eslint-plugin-jsx-a11y": "^6.10.2",
  "eslint-plugin-n": "^17.14.0",
  "eslint-plugin-package-json": "^0.26.0",
  "eslint-plugin-react": "^7.37.2",
  "eslint-plugin-react-hooks": "^5.0.0",
  "eslint-plugin-sonarjs": "^3.0.1",
  "eslint-plugin-unicorn": "^56.0.1",
  "eslint-plugin-unused-imports": "^4.1.4",
  "eslint-plugin-yml": "^1.16.0",
  globals: "^15.14.0",
  neostandard: "^0.11.9",
  "typescript-eslint": "^8.18.0"
};
const devDependencies = {
  "builtin-modules": "^4.0.0",
  eslint: "^9.17.0",
  vitest: "^2.1.5"
};
const peerDependencies = {
  eslint: ">=9"
};
const pkg = {
  name,
  version,
  description,
  main,
  module,
  exports,
  sideEffects,
  files,
  scripts,
  author,
  repository,
  homepage,
  keywords,
  license,
  dependencies,
  devDependencies,
  peerDependencies
};
export {
  author,
  pkg as default,
  dependencies,
  description,
  devDependencies,
  exports,
  files,
  homepage,
  keywords,
  license,
  main,
  module,
  name,
  peerDependencies,
  repository,
  scripts,
  sideEffects,
  version
};

Copy link
Contributor

@byhhh2 byhhh2 left a comment

Choose a reason for hiding this comment

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

LGTM~~ 감사합니다 👍

Copy link
Contributor

@yceffort-naver yceffort-naver left a comment

Choose a reason for hiding this comment

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

감사합니다!!

@keemhyunseok keemhyunseok merged commit 598cf28 into main Mar 11, 2025
3 checks passed
@keemhyunseok keemhyunseok deleted the repo/94_pite branch March 11, 2025 08:10
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.

5 participants