lexical icon indicating copy to clipboard operation
lexical copied to clipboard

[examples/react-rich] Fix: Invalid background-image url path cause missing Toolbar format icons in Vite build bundle.

Open smartappcoder opened this issue 1 year ago • 6 comments

Description

Current vite build used by yarn build does not bundle Toolbar format icon SVG files. This causes missing Toolbar format icons in production build. This is caused by invalid background-image url path in i.undo and other Toolbar format icon selectors in 'src/styles.css'. The fix is changing those urls from

i.undo { background-image: url(icons/arrow-counterclockwise.svg); }

to set to root directory

i.undo { background-image: url(/icons/arrow-counterclockwise.svg); }

Please see more on Static Asset Handling The public Directory section.

Test Plan

  • Setup: clone the lexical repo and run the react-rich

git clone https://github.com/facebook/lexical.git cd examples/react-rich yarn yarn dev

  • The example runs as expected with Toolbar format icons.

react-rich-expected

Before

  • Now run yarn build. The command will complain like this
icons/arrow-counterclockwise.svg referenced in /XXX/lexicail/examples/react-rich/src/styles.css didn't resolve at build time, it will remain unchanged to be resolved at runtime
. . .
  • yarn preview shows missing Toolbar format icons in RichText example.

react-rich-example-missing-formta-icons

  • Same errors happen to static bundle in dist.

After

  • Changes background-iamge url in src/styles.css.
  • Run yarn dev. react-rich run as expected.
  • Rerun yarn build. The command run successfully without complaints.
  • Now yarn preview and dist bundle all have expected Toolbar format icons.

react-rich-expected

smartappcoder avatar May 08 '24 18:05 smartappcoder

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 9, 2024 4:33am
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 9, 2024 4:33am

vercel[bot] avatar May 08 '24 18:05 vercel[bot]

I think a better fix is to move the icons folder from public to src, then the relative paths in the css will work and vite will bundle them (mostly inline as data urls)

etrepum avatar May 08 '24 22:05 etrepum

I think a better fix is to move the icons folder from public to src, then the relative paths in the css will work and vite will bundle them (mostly inline as data urls)

I am OK to move to src/assets or even react-icons or @fontawsome. However that will impact the project structure, which IMHO might need more inputs, arbitrates, and tests.

I suggest to consider this fix as it's impact is isolated. At the same time, I suggest open an enhancement Issue which may include data urls as you suggest, or react-icons or @fontawesome packages.

smartappcoder avatar May 09 '24 02:05 smartappcoder

If you mv public/icons src/icons it just works as-is, without changing any code

etrepum avatar May 09 '24 03:05 etrepum

If you mv public/icons src/icons it just works as-is, without changing any code

Sure, I can do that.

smartappcoder avatar May 09 '24 04:05 smartappcoder

Either approach should be OK as the fix is straightforward. My suggestion is to follow Lexical team's suggested static assets handling for React apps.

My two cents,

react-rich<./pre> is actually 
react-rich-with-vite
. As React suggests Next.js, Remix, Getsby, Expo etc for quickstart, Lexical may plan examples for other React frameworks as well.

See React suggested quick start frameworks Start a New React Project

smartappcoder avatar May 13 '24 17:05 smartappcoder

Unfortunately I can't merge this until the request for changes review is dismissed, if you'd like to submit this fix as a new PR please do so.

etrepum avatar Dec 02 '24 19:12 etrepum