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

Fix from jsmod import * #3903

Merged
merged 11 commits into from Jun 12, 2023
Merged

Conversation

hoodmane
Copy link
Member

@hoodmane hoodmane commented Jun 5, 2023

Prior to this PR:

pyodide.registerJsModule("xx", {a: 2});
pyodide.runPython("from xx import *");

raises:

ImportError: from-import-* object has no __dict__ and no __all__

Afterwards, it behaves as expected (a variable called "a" is introduced into globals equal to 2).

cc @JeffersGlass

src/js/pyodide.ts Outdated Show resolved Hide resolved
@JeffersGlass
Copy link
Contributor

JeffersGlass commented Jun 9, 2023

It's a little surprising to me how few names from js import * imports in the Browser (~226 in Chromium 113):

original_print = print
from js import *
original_print(dir())
['__annotations__', '__builtins__', '__doc__', '__loader__', '__name__', '__package__', '__spec__', '_createPyodideModule', '_pyodide_core', 'alert', 'atob', 'blur', 'btoa', 'caches', 'cancelAnimationFrame', 'cancelIdleCallback', 'captureEvents', 'chrome', 'clearInterval', 'clearTimeout', 'clientInformation', 'close', 'closed', 'confirm', 'cookieStore', 'createImageBitmap', 'credentialless', 'crossOriginIsolated', 'crypto', 'customElements', 'devicePixelRatio', 'document', 'external', 'fetch', 'find', 'focus', 'frameElement', 'frames', 'getComputedStyle', 'getScreenDetails', 'getSelection', 'history', 'indexedDB', 'innerHeight', 'innerWidth', 'isSecureContext', 'launchQueue', 'length', 'loadPyodide', 'localStorage', 'location', 'locationbar', 'main', 'matchMedia', 'menubar', 'moveBy', 'moveTo', 'name', 'navigation', 'navigator', 'onabort', 'onafterprint', 'onanimationend', 'onanimationiteration', 'onanimationstart', 'onappinstalled', 'onauxclick', 'onbeforeinput', 'onbeforeinstallprompt', 'onbeforematch', 'onbeforeprint', 'onbeforeunload', 'onbeforexrselect', 'onblur', 'oncancel', 'oncanplay', 'oncanplaythrough', 'onchange', 'onclick', 'onclose', 'oncontentvisibilityautostatechange', 'oncontextlost', 'oncontextmenu', 'oncontextrestored', 'oncuechange', 'ondblclick', 'ondevicemotion', 'ondeviceorientation', 'ondeviceorientationabsolute', 'ondrag', 'ondragend', 'ondragenter', 'ondragleave', 'ondragover', 'ondragstart', 'ondrop', 'ondurationchange', 'onemptied', 'onended', 'onerror', 'onfocus', 'onformdata', 'ongotpointercapture', 'onhashchange', 'oninput', 'oninvalid', 'onkeydown', 'onkeypress', 'onkeyup', 'onlanguagechange', 'onload', 'onloadeddata', 'onloadedmetadata', 'onloadstart', 'onlostpointercapture', 'onmessage', 'onmessageerror', 'onmousedown', 'onmouseenter', 'onmouseleave', 'onmousemove', 'onmouseout', 'onmouseover', 'onmouseup', 'onmousewheel', 'onoffline', 'ononline', 'onpagehide', 'onpageshow', 'onpause', 'onplay', 'onplaying', 'onpointercancel', 'onpointerdown', 'onpointerenter', 'onpointerleave', 'onpointermove', 'onpointerout', 'onpointerover', 'onpointerrawupdate', 'onpointerup', 'onpopstate', 'onprogress', 'onratechange', 'onrejectionhandled', 'onreset', 'onresize', 'onscroll', 'onsearch', 'onsecuritypolicyviolation', 'onseeked', 'onseeking', 'onselect', 'onselectionchange', 'onselectstart', 'onslotchange', 'onstalled', 'onstorage', 'onsubmit', 'onsuspend', 'ontimeupdate', 'ontoggle', 'ontransitioncancel', 'ontransitionend', 'ontransitionrun', 'ontransitionstart', 'onunhandledrejection', 'onunload', 'onvolumechange', 'onwaiting', 'onwebkitanimationend', 'onwebkitanimationiteration', 'onwebkitanimationstart', 'onwebkittransitionend', 'onwheel', 'open', 'openDatabase', 'opener', 'origin', 'originAgentCluster', 'original_print', 'outerHeight', 'outerWidth', 'pageXOffset', 'pageYOffset', 'parent', 'performance', 'personalbar', 'postMessage', 'print', 'prompt', 'queryLocalFonts', 'queueMicrotask', 'releaseEvents', 'reportError', 'requestAnimationFrame', 'requestIdleCallback', 'resizeBy', 'resizeTo', 'scheduler', 'screen', 'screenLeft', 'screenTop', 'screenX', 'screenY', 'scroll', 'scrollBy', 'scrollTo', 'scrollX', 'scrollY', 'scrollbars', 'self', 'sessionStorage', 'setInterval', 'setTimeout', 'showDirectoryPicker', 'showOpenFilePicker', 'showSaveFilePicker', 'speechSynthesis', 'status', 'statusbar', 'stop', 'structuredClone', 'styleMedia', 'toolbar', 'top', 'trustedTypes', 'visualViewport', 'webkitCancelAnimationFrame', 'webkitRequestAnimationFrame', 'webkitRequestFileSystem', 'webkitResolveLocalFileSystemURL', 'window']

At least compared to all the attributes of the js (globalThis) object generally (~1100 actual attributes; dir below also outputs all the dunder attributes etc):

import js
print(dir(js))
['AbortController', 'AbortSignal', 'AbsoluteOrientationSensor', 'AbstractRange', 'Accelerometer', 'AggregateError', 'AnalyserNode', 'Animation', 'AnimationEffect', 'AnimationEvent', 'AnimationPlaybackEvent', 'AnimationTimeline', 'Array', 'ArrayBuffer', 'Atomics', 'Attr', 'Audio', 'AudioBuffer', 'AudioBufferSourceNode', 'AudioContext', 'AudioData', 'AudioDecoder', 'AudioDestinationNode', 'AudioEncoder', 'AudioListener', 'AudioNode', 'AudioParam', 'AudioParamMap', 'AudioProcessingEvent', 'AudioScheduledSourceNode', 'AudioSinkInfo', 'AudioWorklet', 'AudioWorkletNode', 'AuthenticatorAssertionResponse', 'AuthenticatorAttestationResponse', 'AuthenticatorResponse', 'BackgroundFetchManager', 'BackgroundFetchRecord', 'BackgroundFetchRegistration', 'BarProp', 'BaseAudioContext', 'BatteryManager', 'BeforeInstallPromptEvent', 'BeforeUnloadEvent', 'BigInt', 'BigInt64Array', 'BigUint64Array', 'BiquadFilterNode', 'Blob', 'BlobEvent', 'Boolean', 'BroadcastChannel', 'BrowserCaptureMediaStreamTrack', 'ByteLengthQueuingStrategy', 'CDATASection', 'CSS', 'CSSAnimation', 'CSSConditionRule', 'CSSContainerRule', 'CSSCounterStyleRule', '...']

I guess all these others are attributes which are not enumerable? Removing the filter function on Object.getOwnPropertyNames(o) in this PR makes from js import * import what I think of as "all the names in the JS global namespace", at least on casual inspection.

Not really an issue so much as an observation. Not that I think users should be encouraged to do from js import * generally, just a slightly-surprising (to me) difference. Or perhaps there's something I'm missing.

@hoodmane
Copy link
Member Author

hoodmane commented Jun 9, 2023

Okay I'll remove the filter. Thanks for the review @JeffersGlass!

Copy link
Member

@ryanking13 ryanking13 left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

src/py/_pyodide/_importhook.py Outdated Show resolved Hide resolved
@hoodmane hoodmane merged commit 1943738 into pyodide:main Jun 12, 2023
37 checks passed
@hoodmane hoodmane deleted the from-jsmod-import-star branch June 12, 2023 16:46
hoodmane added a commit to hoodmane/pyodide that referenced this pull request Jul 7, 2023
Prior to this commit:
```js
pyodide.registerJsModule("xx", {a: 2});
pyodide.runPython("from xx import *");
```
raises:
```
ImportError: from-import-* object has no __dict__ and no __all__
```
Afterwards, it behaves as expected (a variable called "a" is introduced into globals equal to 2).
hoodmane added a commit that referenced this pull request Jul 7, 2023
Prior to this commit:
```js
pyodide.registerJsModule("xx", {a: 2});
pyodide.runPython("from xx import *");
```
raises:
```
ImportError: from-import-* object has no __dict__ and no __all__
```
Afterwards, it behaves as expected (a variable called "a" is introduced into globals equal to 2).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants