Skip to content

test(optimize-missing-deps): rewrite deps in CJS#8025

Merged
patak-cat merged 1 commit intovitejs:mainfrom
ygj6:fix-tests
May 5, 2022
Merged

test(optimize-missing-deps): rewrite deps in CJS#8025
patak-cat merged 1 commit intovitejs:mainfrom
ygj6:fix-tests

Conversation

@ygj6
Copy link
Member

@ygj6 ygj6 commented May 5, 2022 •
edited
Loading

Description

Fix test optimize-missing-deps, rewrite deps with CJS. This will pass #6698 failing test and eliminate the following warning:
image

Additional context

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.

@ygj6 ygj6 added the test label May 5, 2022
@ygj6 ygj6 mentioned this pull request May 5, 2022
Closed
9 tasks
@patak-cat
Copy link
Member

patak-cat commented May 5, 2022

Would you explain why the current esm wasnt working? Could it break user code or is it just an artifact of our test setup?

@ygj6
Copy link
Member Author

ygj6 commented May 5, 2022 •
edited
Loading

Would you explain why the current esm wasnt working?

Because we don't have field type: "module" in missing-dep's package.json, if we externalize it, Vite treats it as CJS that raised the error. While we could add this field to package.json, when I try it, I found that jest does not support running ESM in Node environment. So I think the solution is make the deps as CJS.

it just an artifact of our test setup?

yes

@patak-cat patak-cat merged commit e32a3a8 into vitejs:main May 5, 2022
@benmccann benmccann mentioned this pull request May 5, 2022
Closed
9 tasks
benmccann added a commit to benmccann/vite that referenced this pull request May 7, 2022
@benmccann
fix: Revert "test(optimize-missing-deps): rewrite deps in CJS (vitejs…
2d0b1c7 
…#8025)"

This reverts commit e32a3a8.
@benmccann benmccann mentioned this pull request May 7, 2022
Merged
9 tasks
benmccann added a commit to benmccann/vite that referenced this pull request May 10, 2022
@benmccann
fix: Revert rewrite deps in CJS (vitejs#8025) and add missing "type":…
b6673fb 
… "module"

This reverts commit e32a3a8.
benmccann added a commit to benmccann/vite that referenced this pull request May 11, 2022
@benmccann
fix: Revert rewrite deps in CJS (vitejs#8025) and add missing "type":…
9e2c5cc 
… "module"

This reverts commit e32a3a8.
patak-cat pushed a commit that referenced this pull request May 11, 2022
@benmccann
fix: revert #8025 and add missing "type": "module" (#8060)
c613f22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bluwy bluwy bluwy approved these changes

Assignees

No one assigned

Labels

test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ygj6 @patak-cat @bluwy