Skip to content
This repository has been archived by the owner on Jun 12, 2023. It is now read-only.

Update per prerelease build #88

Merged
merged 8 commits into from
Jan 10, 2022
Merged

Conversation

rekmarks
Copy link
Contributor

This PR updates filsnap for compatibility with the MetaMask Flask prerelease build. One usage of nf-snap remains during build time, as builds fail with the latest version of @metamas/snaps-cli due to what appears to be a comment stripping issue. The nature of this issue should be identified and a fix PR'd against the snaps-cli package in this monorepo. Otherwise, we should be good to go!

Comment on lines +9 to 11
// Remove anonymous arrow function wrapper injected by mm-snap
bundleString = bundleString.replace(/\(\) => \(\n/u, '');
bundleString = bundleString.slice(0, bundleString.lastIndexOf('\n)'));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we do this anymore, worth looking at.

@MakMuftic
Copy link
Contributor

@rekmarks Here is the fix for stripping comments https://github.com/MetaMask/snaps-skunkworks/pull/187/files

rekmarks added a commit to MetaMask/snaps that referenced this pull request Jan 7, 2022
### Description
Based on the discussion ChainSafe/filsnap#88 and some further investigation, I realized that the problem origin is inside the library used for stripping comments. The problem occurs if there are double quotes inside the comment, and this issue has been created inside `strip-comments` lib (jonschlinkert/strip-comments#49). Unfortunately, this PR with the fix is stale for some time now, so inside our fork of snaps-cli we used a forked version of `strip-comments` library.

### Changes
In this PR I replaced `strip-comments` with our [fork](https://github.com/NodeFactoryIo/strip-comments) that:
 - fixes a bug with double quotes inside the comment
 - already contains types, so no need for importing them

Co-authored-by: Mak Muftic <mak@chainsafe.io>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants