- Notifications
You must be signed in to change notification settings - Fork1.8k
fix: support windows absolute extends#3062
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
fix: support windows absolute extends#3062
Uh oh!
There was an error while loading.Please reload this page.
Conversation
7444fcf tod8c7d60CompareThere 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.
just one thought wondering if we could clean things up furhter. i havent dug in to better understand if that is a reasonable follow up or not, but i'm going to move this forward and we can do that as a separate PR if it does make sense.
thanks for sticking with this@sheerlox!
| import{castArray,isNil,isPlainObject,isString,pickBy}from"lodash-es"; | ||
| import{readPackageUp}from"read-pkg-up"; | ||
| import{cosmiconfig}from"cosmiconfig"; | ||
| importresolveFromfrom"resolve-from"; |
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.
is resolve-from still used elsewhere in the project? would it be possible to remove it as a dependency?
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.
Yes it is still being used in theloadPlugin function, and we can't replace it withimport-from-esm it because doesn't support ESM named exports (it only returns the whole moduleif thedefault export does not exist).
At least this part of the code is not subject to the Windows absolute path issue since theimport statement explicitly specifies thefile:// protocol.
semantic-release/lib/plugins/utils.js
Lines 50 to 67 ind3aae07
| exportasyncfunctionloadPlugin({ cwd},name,pluginsPath){ | |
| constbasePath=pluginsPath[name] | |
| ?dirname(resolveFrom.silent(__dirname,pluginsPath[name])||resolveFrom(cwd,pluginsPath[name])) | |
| :__dirname; | |
| if(isFunction(name)){ | |
| returnname; | |
| } | |
| constfile=resolveFrom.silent(basePath,name)||resolveFrom(cwd,name); | |
| const{default:cjsExport, ...esmNamedExports}=awaitimport(`file://${file}`); | |
| if(cjsExport){ | |
| returncjsExport; | |
| } | |
| returnesmNamedExports; | |
| } |
🎉 This PR is included in version 22.0.8 🎉 The release is available on: Yoursemantic-release bot 📦🚀 |
Uh oh!
There was an error while loading.Please reload this page.
Description
Restore support for Windows absolute path in the
extendsconfiguration option by usingimport-from-esmto load the referenced module.Context
Following the changes in#3037,a user encountered an
ERR_UNSUPPORTED_ESM_URL_SCHEMEerror becauseusingresolve-fromdoes not return an URL, andimportthen considersC:/to be an URL scheme (likefile://) and returns the encountered error.How has this been tested
Since there are no Windows-specific tests on this repository and the test pipeline doesn't run on Windows, we can rely on
import-from-esmsolid test suite to guarantee this works:index.json& package main entry point JSON modulesis now supported andcovered by testsRelated issues