-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Partially-complete support for "import" with JSObjectReference. #25198
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,6 +61,17 @@ export module DotNet { | |
| 0: new JSObject(window, 0), | ||
| }; | ||
|
|
||
| cachedJSObjectsById[0]._cachedFunctions.set('import', (url: any) => { | ||
| // In most cases developers will want to resolve dynamic imports relative to the base HREF. | ||
| // However since we're the one calling the import keyword, they would be resolved relative to | ||
| // this framework bundle URL. Fix this by providing an absolute URL. | ||
| if (typeof url === 'string' && url.startsWith('./')) { | ||
| url = document.baseURI + url.substr(2); | ||
| } | ||
|
|
||
| return import(/* webpackIgnore: true */ url); | ||
|
Member
Author
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. The |
||
| }); | ||
|
|
||
| let nextAsyncCallId = 1; // Start at 1 because zero signals "no response needed" | ||
| let nextJsObjectId = 1; // Start at 1 because zero is reserved for "window" | ||
|
|
||
|
|
@@ -111,13 +122,13 @@ export module DotNet { | |
|
|
||
| /** | ||
| * Creates a JavaScript object reference that can be passed to .NET via interop calls. | ||
| * | ||
| * | ||
| * @param jsObject The JavaScript Object used to create the JavaScript object reference. | ||
| * @returns The JavaScript object reference (this will be the same instance as the given object). | ||
| * @throws Error if the given value is not an Object. | ||
| */ | ||
| export function createJSObjectReference(jsObject: any): any { | ||
| if (jsObject instanceof Object) { | ||
| if (jsObject && typeof jsObject === 'object') { | ||
|
Member
Author
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. This change is needed because an ES6 module isn't |
||
| if (!jsObject.hasOwnProperty(JSObject.ID_KEY) || cachedJSObjectsById[nextJsObjectId] !== jsObject) { | ||
|
Member
Author
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. Currently this line throws when However, as per my comments here, I don't think we should be doing this stuff with |
||
| // The return value is not cached as a JSObjectReference, or is copied from an object that was, so we cache it as a new one | ||
| cachedJSObjectsById[nextJsObjectId] = new JSObject(jsObject, nextJsObjectId); | ||
|
|
@@ -132,7 +143,7 @@ export module DotNet { | |
|
|
||
| /** | ||
| * Disposes the JavaScript object reference associated with the given object. | ||
| * | ||
| * | ||
| * @param jsObject The JavaScript Object associated with the JavaScript object reference. | ||
| */ | ||
| export function disposeJSObjectReference(jsObject: any): void { | ||
|
|
@@ -260,7 +271,7 @@ export module DotNet { | |
|
|
||
| /** | ||
| * Disposes the JavaScript object reference with the specified object ID. | ||
| * | ||
| * | ||
| * @param id The ID of the JavaScript object reference. | ||
| */ | ||
| disposeJSObjectReferenceById, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,8 @@ | |
| "lib": ["es2015", "dom", "es2015.promise"], | ||
| "strict": true, | ||
| "declaration": true, | ||
| "outDir": "dist" | ||
| "outDir": "dist", | ||
| "module": "ESNext", | ||
|
Member
Author
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. This is needed because otherwise the TypeScript compiler interferes with |
||
| }, | ||
| "include": [ | ||
| "src/**/*.ts" | ||
|
|
||
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 did consider auto-prefixing regular base-relative URIs (e.g.,
myfile.js) with the base URI, but am reluctant to do that because we can't predict other possible syntaxes that browsers might add in the future.However it is safe to rewrite URLs starting with
./because they are unambiguous - we know for sure what that means, and there's no use case for./meaning relative to anything other than the base URI since at runtime the component itself doesn't have any location, and all our other URL-resolution behaviors are base relative too.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'm not sure what this bit means, can you give an explicit example?