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

feat(import-bundle): Support endoScript #2398

Merged
merged 3 commits into from
Aug 31, 2024
Merged

Conversation

kriskowal
Copy link
Member

Description

Adds support for endoScript to the importBundle runtime.

Security Considerations

None.

Scaling Considerations

None.

Documentation Considerations

This change makes safe the assumption that all bundles produced by bundleSource can be consumed by importBundle.

  • NEWS
  • README

Testing Considerations

Unit tests included.

Compatibility Considerations

None.

Upgrade Considerations

None.

Copy link
Contributor

@warner warner left a comment

Choose a reason for hiding this comment

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

LGTM, modulo the two questions

const result = compartment.evaluate(source);
if (moduleFormat === 'endoScript') {
// The completion value of an 'endoScript' is the namespace.
// This format does curry the filePrefix.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/does/does not/ ? or did you mean the format holds the filePrefix internally somehow?

inescapableGlobalValue: 42,
},
});
t.is(ns.default, 42);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not really part of this PR, but should these tests exercise the "inescapability" of inescapableGlobalProperties ? Or did we decide that only the lexical-scope additions needed to be inescapable?

Copy link
Contributor

Choose a reason for hiding this comment

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

looks like transforms being inescapable might be important too

Copy link
Member Author

Choose a reason for hiding this comment

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

The test fixture is a module that runs in a Compartment and creates a child Compartment without options. So, all of these are verifying inescapability at least one level down.

export default new Compartment().evaluate('inescapableGlobalValue');

@kriskowal kriskowal merged commit 0e74a70 into master Aug 31, 2024
15 checks passed
@kriskowal kriskowal deleted the kriskowal-import-endo-script branch August 31, 2024 04:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants