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

compartment-mapper: Temporarily disable CommonJS #500

Merged
merged 5 commits into from
Oct 23, 2020
Merged

Conversation

kriskowal
Copy link
Member

This change temporarily removes CommonJS support from Compartment Mapper to relieve a dependency on Babel that entrained a Node.js builtin, which in turn did not compose well between Rollup and RESM. This should unblock integration in Agoric SDK.

Continued work would begin with reverting this change and adding a CommonJS static analyzer based on a small and fast lexer that captures both imports and exports metadata, consistent with the lexer used by Node.js for exports analysis.

https://github.com/Agoric/SES-shim/compare/kris/restore-cm-cjs

Copy link
Contributor

@erights erights left a comment

Choose a reason for hiding this comment

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

LGTM

};

const fixtureAssertionCount = 11;
const fixtureAssertionCount = 6;
Copy link
Contributor

Choose a reason for hiding this comment

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

Ever more eager for ava, to make this t.plan stuff go away. These manually maintained numbers are awful.

Just editorial venting. No change needed to this PR.

@kriskowal kriskowal force-pushed the kris/temp-no-cjs branch 3 times, most recently from c0aaac4 to b01ee06 Compare October 23, 2020 04:22
@kriskowal kriskowal merged commit 3cf9499 into master Oct 23, 2020
@kriskowal kriskowal deleted the kris/temp-no-cjs branch October 23, 2020 04:48
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