Skip to content

chore: use cjs extension with scripts#5877

Merged
antfu merged 2 commits into
vitejs:mainfrom
benmccann:cjs-exts
Nov 29, 2021
Merged

chore: use cjs extension with scripts#5877
antfu merged 2 commits into
vitejs:mainfrom
benmccann:cjs-exts

Conversation

@benmccann
Copy link
Copy Markdown
Collaborator

@benmccann benmccann commented Nov 28, 2021

Description

Use .cjs extension with Node.js scripts

Additional context

I was looking at what would be required to set "type": "module". This is a change that can be broken off easily and done ahead of time in a way that works with or without setting "type": "module"

Longer-term we'll probably want to convert these to ESM, but that'd make the migration more of a big bang and it'd probably be easier to do it incrementally


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@Shinigami92 Shinigami92 self-requested a review November 28, 2021 17:48
patak-cat
patak-cat previously approved these changes Nov 28, 2021
bluwy
bluwy previously approved these changes Nov 29, 2021
@benmccann benmccann dismissed stale reviews from bluwy and patak-cat via 85343c1 November 29, 2021 03:31
@benmccann
Copy link
Copy Markdown
Collaborator Author

I pushed an update to rename all files in the scripts directories. I hadn't done this earlier because the tests were failing locally, but I've discovered that happens for me even on main and is unrelated to this change

Copy link
Copy Markdown
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

question: Would it be better to use esno and use TS files for most of these files?
That way it could be a bit better typed and therefore easier for contributors to work with.

@Shinigami92 Shinigami92 added the p1-chore Doesn't change code behavior (priority) label Nov 29, 2021
@antfu antfu merged commit 775baba into vitejs:main Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p1-chore Doesn't change code behavior (priority)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants