-
Notifications
You must be signed in to change notification settings - Fork 990
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
chore(esm): convert @redwoodjs/cli-helpers
to ESM
#9872
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jtoar
force-pushed
the
ds-esm/cli-helpers
branch
from
January 23, 2024 00:10
dda4f42
to
965709b
Compare
This reverts commit e4875a9.
Looks like one of the tests isn't handling paths well on Windows. TBD |
Tobbe
reviewed
Jan 23, 2024
Tobbe
reviewed
Jan 23, 2024
Tobbe
reviewed
Jan 24, 2024
|
||
;(Listr as jest.MockedFunction<jest.Mock>).mockImplementation(() => { | ||
;(Listr as vi.MockedFunction<vi.Mock>).mockImplementation(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future, when someone cleans up our testing, this should be fixed. There are much better solutions than type-casting to use here.
Nothing to do now
Tobbe
approved these changes
Jan 24, 2024
dac09
added a commit
to dac09/redwood
that referenced
this pull request
Jan 25, 2024
…dleware * 'main' of github.com:redwoodjs/redwood: chore(structure): switch to vitest (redwoodjs#9878) chore(cli): switch to vitest (redwoodjs#9863) feat(dbAuth): Refactor dbAuthHandler to support WebAPI Request events (redwoodjs#9835) fix(crwa): remove yarn-install option for yarn 1 (redwoodjs#9881) chore(esbuild): dedupe esbuild config (redwoodjs#9875) chore(esm): convert `@redwoodjs/cli-helpers` to ESM (redwoodjs#9872) fix(studio): Add version checks when first running Studio (redwoodjs#9876)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR converts
@redwoodjs/cli-helpers
to a dual ESM/CJS package like #9870 did for@redwoodjs/project-config
. I didn't do anything differently so see that PR for details.As soon as I converted it, the Jest tests stopped working so @Josh-Walker-GM and I bundled both changes into one PR here.
With this PR, we should be able to get all the tests in #9863 passing (about ~15 (out of 1000+) are skipped right now) because Vitest will be able to mock
fs
since it's imported instead of required. That'll be really nice since converting the CLI to ESM will then be possible. I'll confirm that after making this PR. Even if they don't this'll still have been worthwhile.