-
Notifications
You must be signed in to change notification settings - Fork 196
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
Add support for package.json
exports
field
#904
Open
sgammon
wants to merge
1
commit into
oracle:master
Choose a base branch
from
elide-dev:feat/js-module-exports
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,7 +61,10 @@ | |
import java.io.IOException; | ||
import java.net.URI; | ||
import java.net.URISyntaxException; | ||
import java.util.HashMap; | ||
import java.util.LinkedList; | ||
import java.util.List; | ||
import java.util.Map; | ||
|
||
import com.oracle.js.parser.ir.Module.ModuleRequest; | ||
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; | ||
|
@@ -98,9 +101,31 @@ public final class NpmCompatibleESModuleLoader extends DefaultESModuleLoader { | |
private static final String INVALID_MODULE_SPECIFIER = "Invalid module specifier: '"; | ||
private static final String UNSUPPORTED_FILE_EXTENSION = "Unsupported file extension: '"; | ||
private static final String UNSUPPORTED_PACKAGE_EXPORTS = "Unsupported package exports: '"; | ||
private static final String INVALID_PACKAGE_EXPORT = "Invalid package export: "; | ||
private static final String UNSUPPORTED_PACKAGE_IMPORTS = "Unsupported package imports: '"; | ||
private static final String UNSUPPORTED_DIRECTORY_IMPORT = "Unsupported directory import: '"; | ||
private static final String INVALID_PACKAGE_CONFIGURATION = "Invalid package configuration: '"; | ||
private static final String EXPORT_TYPE_GRAALJS = "graaljs"; | ||
private static final String EXPORT_TYPE_IMPORT = "import"; | ||
private static final String EXPORT_TYPE_REQUIRE = "require"; | ||
private static final String EXPORT_TYPE_DEFAULT = "default"; | ||
private static final LinkedList<String> EXPORT_TYPES; | ||
|
||
static { | ||
EXPORT_TYPES = new LinkedList<>( | ||
List.of(EXPORT_TYPE_GRAALJS, EXPORT_TYPE_IMPORT, EXPORT_TYPE_REQUIRE, EXPORT_TYPE_DEFAULT) | ||
); | ||
} | ||
|
||
public static void registerPreferredExportType(String exportType) { | ||
if (!EXPORT_TYPES.contains(exportType)) { | ||
EXPORT_TYPES.addFirst(exportType); | ||
} | ||
} | ||
|
||
public static List<String> getRegisteredExportTypes() { | ||
return List.copyOf(EXPORT_TYPES); | ||
} | ||
|
||
public static NpmCompatibleESModuleLoader create(JSRealm realm) { | ||
return new NpmCompatibleESModuleLoader(realm); | ||
|
@@ -340,6 +365,10 @@ private Format esmFileFormat(URI url, TruffleLanguage.Env env) { | |
if (url.getPath().endsWith(JS_EXT)) { | ||
return Format.ESM; | ||
} | ||
} else if (url.getPath().endsWith(JS_EXT)) { | ||
// Np Fallback to CJS as below (in the case that there is a package.json without a "type" field, or | ||
// the "type" field is not "module"). | ||
return Format.CommonJS; | ||
} | ||
} else if (url.getPath().endsWith(JS_EXT)) { | ||
// Np Package.json with .js extension: try loading as CJS like Node.js does. | ||
|
@@ -349,6 +378,27 @@ private Format esmFileFormat(URI url, TruffleLanguage.Env env) { | |
throw fail(UNSUPPORTED_FILE_EXTENSION, url.toString()); | ||
} | ||
|
||
private URI exportForImport(URI packageUrl, Map<String, String> exports, TruffleLanguage.Env env) { | ||
// in order of preference, find the best import to use for this circumstance; this will be `graaljs` if | ||
// specified (as top preference), then `import`, then `require`, then `default`. if the developer has registered | ||
// their own preferred export types, these will be preferred first. | ||
// | ||
// this branch only activates if package exports are present and need to be used to resolve an import. thus, | ||
// there is no fallback behavior waiting for us, and so an exception is thrown if no export can be matched. | ||
|
||
// 1. for preferred export types... | ||
for (String preferred : getRegisteredExportTypes()) { | ||
// 1.1: is it specified within the exports? | ||
if (exports.containsKey(preferred)) { | ||
// 1.2: if so, resolve the import from the package root. make sure to slice off the `./` prefix. | ||
return packageUrl.resolve(exports.get(preferred).substring(2)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a check that the string actually starts with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, will add |
||
} | ||
} | ||
|
||
// 2. if no preferred export types are specified, or none of them are found, throw an exception. | ||
throw failMessage(UNSUPPORTED_PACKAGE_EXPORTS); | ||
} | ||
|
||
/** | ||
* PACKAGE_RESOLVE(packageSpecifier, parentURL). | ||
*/ | ||
|
@@ -424,6 +474,12 @@ private URI packageResolve(String packageSpecifier, URI parentURL, TruffleLangua | |
PackageJson pjson = readPackageJson(packageUrl, env); | ||
// 11.5 If pjson is not null and pjson.exports is not null or undefined, then | ||
if (pjson != null && pjson.hasExportsProperty()) { | ||
var exp = pjson.getExport(packageSubpath); | ||
if (exp != null) { | ||
// we should receive a map of the form `type => path` for the requested export. determine the best | ||
// import type to use and resolve from there. | ||
return exportForImport(packageUrl, exp, env); | ||
} | ||
throw fail(UNSUPPORTED_PACKAGE_EXPORTS, packageSpecifier); | ||
} else if (packageSubpath.equals(DOT)) { | ||
// 11.6 Otherwise, if packageSubpath is equal to ".", then | ||
|
@@ -544,6 +600,54 @@ public boolean hasExportsProperty() { | |
return hasNonNullProperty(jsonObj, EXPORTS_PROPERTY_NAME); | ||
} | ||
|
||
public Map<String, String> getExport(String specifier) { | ||
assert hasNonNullProperty(jsonObj, EXPORTS_PROPERTY_NAME); | ||
var data = JSObject.get(jsonObj, EXPORTS_PROPERTY_NAME); | ||
if (data instanceof JSDynamicObject exportsObj) { | ||
for (TruffleString key : JSObject.enumerableOwnNames(exportsObj)) { | ||
// find a match for the requested export... | ||
if (key.toString().equals(specifier)) { | ||
// if we found it, it should be a nested object with export mappings. at this point, we've | ||
// already matched the path, so these are mappings of (type => path). `path` must be relative to | ||
// the package root, must start with `.`, must not contain relative backwards references, and | ||
// must be an extant regular file. | ||
Object value = JSObject.get(exportsObj, key); | ||
if (value instanceof JSDynamicObject valueObj) { | ||
var exportKeys = valueObj.ownPropertyKeys(); | ||
var exportMap = new HashMap<String, String>(); | ||
for (Object exportKey : exportKeys) { | ||
if (exportKey instanceof TruffleString exportKeyStr) { | ||
Object exportValue = JSObject.get(valueObj, exportKeyStr); | ||
if (Strings.isTString(exportValue)) { | ||
var exportStr = exportKeyStr.toString(); | ||
var exportVal = exportValue.toString(); | ||
if (!exportVal.startsWith(".") || exportVal.contains("..")) { | ||
// must start with `.`, must not contain `..` | ||
throw failMessage(INVALID_PACKAGE_EXPORT + exportStr); | ||
} | ||
exportMap.put(exportKeyStr.toString(), exportValue.toString()); | ||
} | ||
} else { | ||
throw failMessage(UNSUPPORTED_PACKAGE_EXPORTS + exportKey.toString()); | ||
} | ||
} | ||
return exportMap; | ||
} else if (value instanceof TruffleString exportStr) { | ||
// if the export is a string, it should be a path to the file to import. | ||
if (!exportStr.toString().startsWith(".") || exportStr.toString().contains("..")) { | ||
// must start with `.`, must not contain `..` | ||
throw failMessage(INVALID_PACKAGE_EXPORT + exportStr); | ||
} | ||
return Map.of(EXPORT_TYPE_DEFAULT, exportStr.toString()); | ||
} else { | ||
throw failMessage(INVALID_PACKAGE_EXPORT + value); | ||
} | ||
} | ||
} | ||
} | ||
return null; | ||
} | ||
|
||
public boolean hasMainProperty() { | ||
if (JSObject.hasProperty(jsonObj, PACKAGE_JSON_MAIN_PROPERTY_NAME)) { | ||
Object value = JSObject.get(jsonObj, PACKAGE_JSON_MAIN_PROPERTY_NAME); | ||
|
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.
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.
Where is this used and for what?
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.
This would be used by embedders:
... would register the engine
elide
as a preferred token, greater in precedence thangraaljs
,import
,require
, ordefault
. Perhaps a browser builder could register"browser"
to prefer imports of that type.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.
I see... could you do this in a way that does not require modifying a
static final
list?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.
@woess I can try; for example, what if the embedder registers a callback to provide a list of candidate strings? I just figure the callback would probably not need to be dynamic in nature (these strings are likely registered once at startup and not again).
I could wrap the list in an atomic, but that wouldn't do much. What would you recommend as an approach for this one?