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

Adds Vite to the Indie Stack #284

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

Adds Vite to the Indie Stack #284

wants to merge 14 commits into from

Conversation

shmuli9
Copy link

@shmuli9 shmuli9 commented May 7, 2024

  • uses Vite and updates code accordingly
  • remove dep on ts-node and replace with vite-node for simpler ESM support

Copy link

Choose a reason for hiding this comment

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

Great work @shmuli9. Just curious why we change .js to .cjs? We did say "type": "module" in package.json I would have thought js will be treated as mjs? Did I miss anything? Thanks.

Copy link
Author

@shmuli9 shmuli9 Aug 10, 2024

Choose a reason for hiding this comment

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

@enRose I can't actually remember the exact reason why this was necessary. Pretty sure it was because eslint doesn't support esm.
If you've applied these changes do you want to check that it works ok as .js?

Copy link

Choose a reason for hiding this comment

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

@shmuli9 I think you are right. Ran it as .js says require() not supported. I wonder if v9 of eslint would actually work with .js.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, pretty sure eslint v9 supports esm natively.

You can see here that for v8.57,.cjs is required

@enRose
Copy link

enRose commented Aug 10, 2024

I have applied these changes as well to my indie stack and it works! Thanks @shmuli9

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