Skip to content

fix(cli): convert Windows absolute paths to file URLs for dynamic imports #7126

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

Merged
merged 2 commits into from
Apr 25, 2025

Conversation

jtmaveryk
Copy link
Contributor

@jtmaveryk jtmaveryk commented Mar 11, 2025

fix(cli): convert Windows absolute paths to file URLs for dynamic imports

The dynamic import in importAlterationScript was failing on Windows because it used an absolute file path without converting it to a valid file URL. This commit updates the code to use pathToFileURL for converting file paths, resolving the ERR_UNSUPPORTED_ESM_URL_SCHEME error during database alteration deployment.

Summary

This change fixes a bug where dynamic imports were failing on Windows due to the use of absolute file paths without the proper file:// scheme. By converting these paths using Node's pathToFileURL, the error is resolved and the deployment process works consistently across platforms.

Testing

Tested on Windows by running the logto db alt deploy command with a sample database URL. Verified that the alterations deploy successfully without triggering the ERR_UNSUPPORTED_ESM_URL_SCHEME error, and confirmed that existing unit tests pass.

Checklist

  • .changeset
  • unit tests
  • integration tests
  • necessary TSDoc comments

Copy link

github-actions bot commented Mar 11, 2025

COMPARE TO master

Total Size Diff 📈 +63 Bytes

Diff by File
Name Diff
packages/cli/src/commands/database/alteration/index.ts 📈 +63 Bytes

Copy link
Contributor

@wangsijie wangsijie left a comment

Choose a reason for hiding this comment

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

LGTM

@charIeszhao
Copy link
Member

Hi, there's actually a typo in the commit message, as the CI complains right now. The commit scope should be cli instead of ci. Would you like to update it?

@jtmaveryk
Copy link
Contributor Author

Hi, there's actually a typo in the commit message, as the CI complains right now. The commit scope should be cli instead of ci. Would you like to update it?

Sure thing!

@jtmaveryk jtmaveryk changed the title fix(ci): convert Windows absolute paths to file URLs for dynamic imports fix(cli): convert Windows absolute paths to file URLs for dynamic imports Mar 31, 2025
Copy link

This PR is stale because it has been open 10 for days with no activity. Remove stale label or comment or this will be closed in 5 days.

@charIeszhao charIeszhao enabled auto-merge (squash) April 25, 2025 10:38
@charIeszhao charIeszhao merged commit ba76210 into logto-io:master Apr 25, 2025
34 checks passed
@jtmaveryk jtmaveryk deleted the patch-1 branch May 10, 2025 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants