Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat!: deprecate cjs node api #14278

Merged
merged 8 commits into from
Sep 21, 2023
Merged

feat!: deprecate cjs node api #14278

merged 8 commits into from
Sep 21, 2023

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Sep 4, 2023

Description

  1. Add warning if CJS node api is used.
  2. Support VITE_CJS_TRACE flag to use console.trace when logging the warning.
  3. Fix CJS types (make empty as suggested in TS errors after switching moduleResolution from Node to Node16 #11552 (comment))

fix #11552

Additional context

@andrewbranch, just confirming, is this PR making the right change to fix Vite's CJS types? I created an empty index.d.cts for CJS exports.


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 PR Title 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.

@bluwy bluwy added the p3-significant High priority enhancement (priority) label Sep 4, 2023
@bluwy bluwy added this to the 5.0 milestone Sep 4, 2023
@stackblitz
Copy link

stackblitz bot commented Sep 4, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@andrewbranch
Copy link

andrewbranch commented Sep 5, 2023

That should make TS error with something like "vite" is not a module. Maybe adding an export {} would be better, so the error will be something like Module "vite" has no export '{whatever name you tried to import}'.

@sapphi-red
Copy link
Member

Would it work if we wrote it like this? It'd be nice to show it with a strikethrough.

/**
 * @deprecate Vite CJS Node API deprecated. See https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated for more details.
 */
declare const module: any

export = module

@andrewbranch
Copy link

I’m not sure. Worth a try. A deprecated any is a little friendlier than a confident nothing, anyway.

sapphi-red
sapphi-red previously approved these changes Sep 10, 2023
@bluwy bluwy merged commit 404f30f into main Sep 21, 2023
@bluwy bluwy deleted the vite-cjs-deprecation branch September 21, 2023 09:01
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
p3-significant High priority enhancement (priority)
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

TS errors after switching moduleResolution from Node to Node16
4 participants