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(compartment-mapper): Work around dynamic import censoring #512

Merged
merged 3 commits into from
Nov 5, 2020

Conversation

kriskowal
Copy link
Member

This change puts a ring of parentheses on calls to import, and inlines type definitions for buffered reader and writer to avoid using import at all in TypeScript JSDoc type annotations.

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

@@ -28,7 +28,17 @@
* comment: string, // presumed UTF-8
* }} CentralDirectoryLocator
*
* @typedef {import('./buffer-reader.js').BufferReader} BufferReader
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh damn. I didn't see this one coming. Unpleasant but I have no suggested improvement.

Copy link
Member Author

Choose a reason for hiding this comment

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

It’s technically an improvement for the type here to express what’s needed since it doesn’t depend on the concrete type. But, in the future, we will need other work-arounds.

Comment on lines +57 to +58
// Wrap import calls to bypass SES censoring for dynamic import.
// eslint-disable-next-line prettier/prettier
Copy link
Member Author

Choose a reason for hiding this comment

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

Added this boilerplate to stop lint --fix from erasing the “unnecessary” parentheses.

@kriskowal kriskowal merged commit b82398b into master Nov 5, 2020
@kriskowal kriskowal deleted the kris-cm-import-censoring branch November 5, 2020 22:17
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

2 participants