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

[DOJO-45] フォルダ構成の整理 (修正1) #35

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

iusok3386
Copy link
Collaborator

@iusok3386 iusok3386 commented Feb 10, 2025

やったこと

  • src ディレクトリへの移行に伴い、不具合が多く発生したので修正しています
    • shadcn の設定を修正 (components.json)
      • pnpm dlx shadcn@latest add [xxx] でコンポーネントを追加できないことがあったのが修正された
    • ESLint の設定を修正 (eslint.config.mjs)
      • src/*.{tsx,ts} のファイルに対して全く動作していなかった
    • 一部ファイルの import の修正ができていなかった
  • package-lock.json の削除
  • pnpm i を実行したら、pnpm-lock.yaml の lockfileVersion6.0 -> 9.0 になった

Copy link
Contributor

@wf9a5m75 wf9a5m75 left a comment

Choose a reason for hiding this comment

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

ありがとうございます。importのパスについては、@/による絶対パスと相対パスが混ざっているので、どちらかに統一するのが良いでしょう。(ちなみに、このインポートのパスについては、皆さん結構こだわりが強い)

@@ -9,6 +9,7 @@ export async function GET() {
try {
const problems = await problemUseCase.getAllProblems();
return NextResponse.json(problems, { status: 200 });
// eslint-disable-next-line @typescript-eslint/no-unused-vars
Copy link
Contributor

Choose a reason for hiding this comment

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

} catch (_error) { のように _ を変数名の先頭につけるのが一般的な方法のはずです。
https://johnnyreilly.com/typescript-eslint-no-unused-vars

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修正しました
8aee38e

import Image from 'next/image';
import Spinner from '@/public/spinner.svg';
import Spinner from '@/../public/spinner.svg';
Copy link
Contributor

Choose a reason for hiding this comment

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

@/ にすることのメリットは、管理がしやすくなることです。

tsconfig.jsonpaths@lib/@public/ を定義するのが良いでしょう

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修正しました
67f9ca9

@@ -2,7 +2,7 @@ import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs';
import { cn } from '@/lib/utils';
import { Code } from 'lucide-react';
import { CodeEditor } from './code-editor';
import { Language, SupportedLanguage } from '@/lib/languages';
import { Language, SupportedLanguage } from '@/../lib/languages';
Copy link
Contributor

Choose a reason for hiding this comment

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

tsconfig.jsonpaths@lib/ を定義するのが良いでしょう

Copy link
Collaborator Author

@iusok3386 iusok3386 Feb 11, 2025

Choose a reason for hiding this comment

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

そもそも、Language/lib/languages.ts で定義したほうがいい気がしてきました。
Strapi からの参照できるように、と思ってルートディレクトリの lib に配置していましたが、プロジェクトの構造としてあまり良くありませんね。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修正しました
932772b

@@ -15,7 +15,7 @@ import { Confirm } from '@/components/ui/alert-dialog';
import { OnMount } from '@monaco-editor/react';
import { DialogPortalProps } from '@radix-ui/react-dialog';
import { Button } from '@/components/ui/button';
import { RestartAltIcon } from './icons/material-symbols';
import { RestartAltIcon } from '@/components/ui/icons';
Copy link
Contributor

Choose a reason for hiding this comment

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

tsconfig.jsopaths@icons を定義するか、
@components/ を定義するのが良いでしょう。

なぜこのようにするかといえば、iconはリソースであり、ソースコードではないので、src/以下にある必要性がない(と考える人達が居る)からです。

コンポーネントを開発したエンジニアとしては、コンポーネントとそのリソースが近くにある方が良い、と考えるかもしれません。

でもデザイナーなら、コードじゃないなら src/以下ではなくて、rootディレクトリ直下にあった方がやりやすいわけです。

そのような意見があるので、柔軟に対応できるように paths で仮想パスを定義するのです

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

なるほど、そのような考え方もあるのですね。
勉強になります!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修正しました
f95bfbe

@iusok3386 iusok3386 requested a review from wf9a5m75 February 11, 2025 14:21
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