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

fix(rome_playground): Unicode Support #2332

Merged
merged 1 commit into from
Mar 31, 2022
Merged

Conversation

MichaReiser
Copy link
Contributor

@MichaReiser MichaReiser commented Mar 31, 2022

They playground crashed because btoa only supports ASCII.
This PR ensures that non ASCII characters are propertly encoded before running the code throuh btoa.

Summary

Test Plan

Screenshot from 2022-03-31 11-38-50

They playground crashed because `btoa` only supports ASCII.
This PR ensures that non ASCII characters are propertly encoded before running the code throuh `btoa`.
@@ -22,7 +22,7 @@ jobs:
node-version: '14'
- run: npm install --prefix crates/rome_playground
- uses: jetli/wasm-pack-action@v0.3.0
- run: wasm-pack build crates/rome_playground --target web
- run: wasm-pack build crates/rome_playground --target web --release
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This reduces the wasm size by 0.5 mb

Copy link
Contributor

Choose a reason for hiding this comment

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

hum... if we have not done anything to reduce, can this help?
https://rustwasm.github.io/book/reference/code-size.html

@@ -2,7 +2,7 @@ import CodeEditor from "@uiw/react-textarea-code-editor";
import "react-tabs/style/react-tabs.css";
import init, { run } from "../pkg/rome_playground";
import { Tabs, Tab, TabList, TabPanel } from "react-tabs";
import {useEffect, useMemo, useState} from "react";
import { useEffect, useMemo, useState } from "react";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nit: We should probably move the playground into the website directory. It isn't a proper "rome" rust crate

Copy link
Contributor

Choose a reason for hiding this comment

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

It's the same thing that I suggested a while ago.

@cloudflare-workers-and-pages
Copy link

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 89c4520
Status: ✅  Deploy successful!
Preview URL: https://2d4d435a.tools-8rn.pages.dev

View logs

@MichaReiser MichaReiser merged commit dad3248 into main Mar 31, 2022
@MichaReiser MichaReiser deleted the fix/playground-unicode branch March 31, 2022 11:48
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