Skip to content

fix initializer: configPath should never be inside /src and should be .js #64

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
danieldelcore merged 1 commit into from
Nov 1, 2021
Merged

fix initializer: configPath should never be inside /src and should be .js #64

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
2 changes: 1 addition & 1 deletion packages/initializer/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export function initDirectory(
const transformPath = `${basePath}${!isReduced ? '/src/' : ''}/${transform}`;
const configPath = `${basePath}${
!isReduced ? '/src' : ''
}/codeshift.config.ts`;
}/codeshift.config.js`;
Copy link
Contributor

@danieldelcore danieldelcore Nov 1, 2021

Choose a reason for hiding this comment

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

Hey @kiprasmel 👋 You should be able to initialise and simply switch over to a js codeshift.config.js file.
I'd prefer to have TS be the default output type to be consistent and typesafe.

Are you able to share more about the issue you're running into? maybe I can help 😄

Copy link
Contributor Author

@kiprasmel kiprasmel Nov 1, 2021

Choose a reason for hiding this comment

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

hey hey, that's fair, but i still think the config being js is a better default.

i am a big typescript fan and have been using it almost for every project for the last few years, but this is different. this could very well be a json file, and we could call the require.resolve ourselves.
it has little to do w/ type safety, and a lot with making tools that could utilize it, easier and simpler, because importing json/js is way easier than uncompiled .ts:D

i have created a wrapper cli around codeshift-cli just for simple running of local files, and having the js file as config makes it easy to import w/o having to compile or run ts-node first. the whole cli is temporary since we only need it for quickly running the local codemods but hopefully after more usage i can upstream the changes here. sadly cannot share atm since it depends on other unrelated code..

Copy link
Contributor

@danieldelcore danieldelcore Nov 1, 2021

Choose a reason for hiding this comment

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

this could very well be a json file, and we could call the require.resolve ourselves.
it has little to do w/ type safety, and a lot with making tools that could utilize it, easier and simpler, because importing json/js is way easier than uncompiled .ts:D

I really like this rationale! I'm onboard!

the whole cli is temporary since we only need it for quickly running the local codemods but hopefully after more usage i can upstream the changes here

Sounds amazing, i'd love that!

Copy link
Contributor Author

@kiprasmel kiprasmel Nov 3, 2021

Choose a reason for hiding this comment

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

fyi, this is an example of why i needed this: #65


if (fs.existsSync(transformPath)) {
throw new Error(`Codemod for ${type} "${transform}" already exists`);
Expand Down