Skip to content

Don't include package import context if pth files compat flag is set#6714

Merged
hoodmane merged 1 commit intomainfrom
hoodmane/no-package-entropy-import-context-if-pth-files-on
May 4, 2026
Merged

Don't include package import context if pth files compat flag is set#6714
hoodmane merged 1 commit intomainfrom
hoodmane/no-package-entropy-import-context-if-pth-files-on

Conversation

@hoodmane
Copy link
Copy Markdown
Contributor

@hoodmane hoodmane commented May 1, 2026

Instead we'll get them from workers-runtime-sdk

@hoodmane hoodmane requested review from a team as code owners May 1, 2026 17:51
@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented May 1, 2026

Now I have a clear picture. Let me analyze the key issue:

In lib.ts line 15, entropyImportContextPackages is imported unconditionally at the top level:

import { default as entropyImportContextPackages } from 'pyodide-internal:topLevelEntropy/entropy_import_context_packages.py';

But it's only used conditionally at line 100-104 (when !PROCESS_PTH_FILES). When PROCESS_PTH_FILES is true, the import is unused — the module is still loaded and its content read, even though it's never mounted to the filesystem.

The Python side handles this by wrapping the import in a try/except ImportError, which correctly handles the case where the file isn't mounted. But the TypeScript side still unconditionally imports the module. This means the module content is still being loaded/bundled even when it won't be used.

However, this is a builtin module import (pyodide-internal:...), not a dynamic file read — it's being bundled into the worker at build time. So the import will always succeed (it's compiled-in), but the data just won't be used when the flag is on. This isn't really a bug — it's just a minor inefficiency (unused import). In the context of how these builtin modules work, this is unlikely to cause errors.

Let me verify there are no other issues:

  1. Compat flag change: Adding $compatEnableDate("2026-05-26") to an existing flag — this is just setting the date when it becomes default. This is a standard, safe operation.

  2. Python try/except: The try/except ImportError is correctly placed and the comment explains why.

  3. TypeScript conditional mount: The if (!PROCESS_PTH_FILES) check correctly skips mounting when the flag is on.

The only potential issue is the unconditional import on line 15 of lib.ts. When PROCESS_PTH_FILES is true, the file entropy_import_context_packages.py won't be written to the filesystem, but the import at line 15 will still load the raw module content into memory. This is wasted memory/work but shouldn't cause a runtime error since it's a builtin module.

Actually, let me reconsider — if the flag is on, will this builtin module still be available? These are embedded at build time, so yes. This is a minor inefficiency, not a bug.

This is a clean, small PR. The logic is correct. The only observation is the unconditional import at line 15 being a minor waste when the flag is enabled, but that's not actionable in the module system being used here (ES module imports are static and can't be conditional).

LGTM

github run

@hoodmane hoodmane force-pushed the hoodmane/no-package-entropy-import-context-if-pth-files-on branch from 88121ad to fc368a8 Compare May 1, 2026 17:57
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 1, 2026

Merging this PR will not alter performance

✅ 72 untouched benchmarks
⏩ 129 skipped benchmarks1


Comparing hoodmane/no-package-entropy-import-context-if-pth-files-on (26ab238) with main (d092243)

Open in CodSpeed

Footnotes

  1. 129 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Instead we'll get them from workers-runtime-sdk
@hoodmane hoodmane force-pushed the hoodmane/no-package-entropy-import-context-if-pth-files-on branch from fc368a8 to 26ab238 Compare May 1, 2026 21:41
@hoodmane hoodmane merged commit a5a0053 into main May 4, 2026
24 checks passed
@hoodmane hoodmane deleted the hoodmane/no-package-entropy-import-context-if-pth-files-on branch May 4, 2026 05:39
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.

2 participants