Skip to content

Fix ejs builds #2632

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix ejs builds #2632

wants to merge 4 commits into from

Conversation

zoriya
Copy link

@zoriya zoriya commented Feb 10, 2025

Summary

The current ejs build includes code generated from pegjs/peggy (idk why both were still used since peggy is the successor of pegjs, i took the liberty to upgrade uses of pegjs to peggy). This generated code uses commonjs exports, which don't work well in some environments (i'm trying to run it in vite.)

This PR changes the generation mode of peggy to ejs & let babel translate those ejs modules to commonjs for the cjs build.

This PR also removes the Buffer import (which is a node only module, so unavailable on the web). This replaces the use of Buffer with window.atob when unavailable.

Test Plan

I just used my build in my project.

What's required for testing (prerequisites)?

Just use the lib

What are the steps to reproduce (after prerequisites)?

Try to import react-native-svg from a vite based project (one built with one for example).

Compatibility

OS Implemented
iOS
MacOS
Android
Web

Checklist

  • I have tested this on a device and a simulator
  • I added documentation in README.md
  • I updated the typed files (typescript)
  • I added a test for the API in the __tests__ folder

@@ -0,0 +1,9 @@
{pkgs ? import <nixpkgs> {}}:
pkgs.mkShell {
packages = with pkgs; [
Copy link
Author

Choose a reason for hiding this comment

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

This file just declares packages needed to work on the project. It's common to upstream this for other users of the nix packet manager. If you don't want it included, I can just remove it. LMK

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.

1 participant