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

chore(cli): convert the cli to ESM #10409

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

chore(cli): convert the cli to ESM #10409

wants to merge 1 commit into from

Conversation

jtoar
Copy link
Contributor

@jtoar jtoar commented Apr 3, 2024

This PR starts on getting the CLI to ESM. The CLI isn't consumed by any other packages it's just invoked so it doesn't have to be dual published.

So far most of the changes are adding file extensions and changing the way we import CJS packages. Some packages that we've told renovate to ignore because they're ESM-only may only be used by the CLI and could be upgraded to their latest versions.

The bin proxies in @redwoodjs/core were tricky since we can't require files from the CLI. That part I'd still like to think more about. (And the const { ... } = pkg workarounds.) CI will tell us if the existing strategy works on Windows.

So far this is one of those PRs where, as I fix one thing, another thing comes up. It looks like I still have to deal with

  • require
  • __dirname
  • commandDir
  • review dist again; there's some files that don't need to be there

@jtoar jtoar added this to the next-release milestone Apr 3, 2024
@jtoar jtoar added the release:chore This PR is a chore (means nothing for users) label Apr 3, 2024
@jtoar
Copy link
Contributor Author

jtoar commented Apr 3, 2024

It looks like we can't use commandDir with ESM which is something I've tried to refactor out before so, it's just time to do it across the board. Source: https://github.com/yargs/yargs/blob/main/docs/advanced.md#commanddirdirectory-opts.

@@ -39,6 +40,8 @@ export const handler = async ({
const useFragments = rwjsConfig.graphql?.fragments
const useTrustedDocuments = rwjsConfig.graphql?.trustedDocuments

const require = createRequire(path.join(getPaths().base, 'node_modules'))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll probably refactor this into a helper once I have a better idea of where it's used

import { timedTelemetry } from '@redwoodjs/telemetry'

import { getPaths, getConfig } from '../lib'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is ESM-specific. Implicit imports like this (this is really '../lib/index.js') don't work

@@ -693,7 +693,7 @@ export const handler = async (yargs) => {
verbose: yargs.verbose,
})

const { NodeSSH } = require('node-ssh')
const { NodeSSH } = await import('node-ssh')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has to be tested still

@Tobbe Tobbe changed the title chore(cli): conver the cli to ESM chore(cli): convert the cli to ESM Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:chore This PR is a chore (means nothing for users)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant