react-native-svg icon indicating copy to clipboard operation
react-native-svg copied to clipboard

SvgXml implementation for web

Open MatheusrdSantos opened this issue 5 years ago • 13 comments
trafficstars

Summary

Explain the motivation for making this change: here are some points to help you:

  • What issues does the pull request solve? Please tag them so that they will get automatically closed once the PR is merged #1279 #1236

  • What is the feature? (if applicable) This PR contains the implementation of SvgXml for web. This web implementation has exactly the same usage as the native implementation.

  • How did you implement the solution? The web implementation relies on unstable_createElement and View from react-native-web. So, basically I used unstable_createElement to create an <svg> element with the props that comes from the XML string and set the innerHTML from the <svg> to be the XML content. In order to accept react-native props (the same as SvgXml native implementation), I used the <View> to wrap the <svg> and simulate the same behavior.

  • What areas of the library does it impact? None. It just brings a new supported component for web.

Test Plan

You can check a working demo here: https://codesandbox.io/s/keen-platform-z40rg

Compatibility

OS Implemented
iOS
Android
Web

Checklist

  • [x] I have tested this on browser

MatheusrdSantos avatar Aug 04 '20 18:08 MatheusrdSantos

I hope release this quickly. I need SvgXml for react native web!!

kofsiwon avatar Aug 18 '20 04:08 kofsiwon

For those still needing this, you can try our package: @cantoo/rn-svg

Sharcoux avatar Jul 11 '23 21:07 Sharcoux

@WoLewicki @MatheusrdSantos @Sharcoux Should we merge this in? It seems risky long term to have a fork maintain this functionality. Thanks!

MattFoley avatar May 28 '24 23:05 MattFoley

@MattFoley I'm glad that the project revived.

We can rebase our fork and make it ready for merge, that would be awesome. However, there is a concern you should know about:

We had to wrap the svg inside a View. This has impacts when trying to style the SVG, especially with everything related to layout. We made a system that distribute the styles between the wrapper and the svg itself, but it will very likely introduce problems in some layouts. unstable_createElement would not let us pass down every react-native props to the underlying element. Another valid approach could be to only export the element created with unstable_createElement and give up on unsupported props.

You'll find all the details here: https://github.com/necolas/react-native-web/pull/1686

Sharcoux avatar May 29 '24 08:05 Sharcoux

Hmm, I'll be honest I don't fully understand the debate on RNW, and would need more time to dive into the specifics. I also am using v14.1 of rn-svg, and don't think I can downgrade to 13. I started to rebase your fork up to v14.1 myself, but haven't finished yet, working through some BoB type errors.

MattFoley avatar May 29 '24 16:05 MattFoley

Ouch, I missed your comment, sorry.

No, don't rebase this here. If you want to follow our approach, we need to push our current version of it. Many fixes have been made since this.

Sharcoux avatar Jun 25 '24 21:06 Sharcoux

Moreover, @MattFoley there should not be any rebase needed because we only need to add new files. Not edit existing ones.

Sharcoux avatar Jun 25 '24 21:06 Sharcoux

@Sharcoux I would love if you had a new version you could push up on 14.x!

MattFoley avatar Jun 25 '24 21:06 MattFoley

Hello, @MatheusrdSantos, could you please explain to me what the difference is between that solution and our current implementation?

Thank you.

bohdanprog avatar Jul 03 '24 14:07 bohdanprog

Hi, @bohdanprog

this PR is just adding the web implementation for the SvgXml component, which is currently missing from the main package. It uses react-native-web to keep the react-native syntax, just as the current web implementation for Svg, G, Path... does.

The main difference is the need to import some hooks that are not exposed by the react-native-web api. Those hooks are important to emulate the native behavior, and therefore, make it possible to watch layout changes, measure the element and listen for touches by using the same native api.

This PR (https://github.com/software-mansion/react-native-svg/pull/1686) has the same implementation I did, but it's not rebased. Anyway it's a good complement to my explanation.

MatheusrdSantos avatar Jul 08 '24 21:07 MatheusrdSantos

Hi @MatheusrdSantos, I’ve improved some parts of your code and hope you don’t mind. Could you take a look at my changes and let me know what you think? Thank you!

bohdanprog avatar Jul 26 '24 10:07 bohdanprog

@bohdanprog The changes are good, thanks for the code improvements.

There's just one change I didn't understand. Is there a reason to move the react-native-web package from peerDependencies/optionalDependencies to dependencies?

I think we could keep this dependency as optional because not all user may want to use the web implementation.

MatheusrdSantos avatar Jul 29 '24 15:07 MatheusrdSantos

Hi @MatheusrdSantos,

We have made some updates to react-native-svg, and as far as I know, we can now display SvgXml, SvgCss, etc. Would you like to verify this? Thank you.

bohdanprog avatar Aug 01 '24 09:08 bohdanprog

I tried. SvgXml is undefined if I import import { SvgXml } from 'react-native-web'. When I check the code, I see that there is an implementation in the file src/xml.tsx. However, I don't understand how that implementation is supposed to be made available for the web, as it is never exported from ReactNativeSVG.web.ts

I think that you want to copy the lines 29 to 47 from ReactNativeSVG.ts into ReactNativeSVG.web.ts:

import {
  parse,
  SvgAst,
  SvgFromUri,
  SvgFromXml,
  SvgUri,
  SvgXml,
  camelCase,
  fetchText,
  JsxAST,
  Middleware,
  Styles,
  UriProps,
  UriState,
  XmlAST,
  XmlProps,
  XmlState,
  AstProps,
} from './xml';

But also the lines 141 to 146 and 178 to 188:

image

export type {
  JsxAST,
  Middleware,
  Styles,
  UriProps,
  UriState,
  XmlAST,
  XmlProps,
  XmlState,
  AstProps,
};

Sharcoux avatar Aug 05 '24 22:08 Sharcoux

Hi @Sharcoux, Can you please try to run the test app from the main branch? Those functionalities are already available if you use the library from the main branch. I believe we are going to create the new release of react-native-svg today.

bohdanprog avatar Aug 06 '24 06:08 bohdanprog

@Sharcoux FYI, now it should work when you are using react-native-svg 15.5.0

bohdanprog avatar Aug 08 '24 06:08 bohdanprog

Closing as it has been implemented in https://github.com/software-mansion/react-native-svg/releases/tag/v15.5.0

jakex7 avatar Aug 12 '24 14:08 jakex7

Hi, @bohdanprog I tested the version 15.5.0. There are two issues.

The first error is happening because the dependency @react-native/assets-registry is missing in the package.json file from react-native-svg. It can be easily fixed by manually installing the dependency.

Module not found: Can't resolve '@react-native/assets-registry/registry' in '/home/matheus/cantoo/cantoo-monorepo/node_modules/react-native-svg/lib/module/lib'
node_modules/react-native-svg/lib/module/lib/resolveAssetUri.js

The second issue is that @react-native/assets-registry uses flow to check its types and doesn't expose a built version for general js usage. There are two open discussions about this issue: fix: Remove Flow from @react-native/assets-registry - registry.js: https://github.com/facebook/react-native/pull/44002 Build assets-registry for general consumption: https://github.com/facebook/react-native/pull/39440

Module parse failed: Unexpected token (13:7)
node_modules/@react-native/assets-registry/registry.js
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
| 'use strict';
| 
> export type PackagerAsset = {
|   +__packager_asset: boolean,
|   +fileSystemLocation: string,

MatheusrdSantos avatar Aug 19 '24 17:08 MatheusrdSantos

@MatheusrdSantos Can you tell me please, how can I reproduce that issue?

bohdanprog avatar Aug 19 '24 17:08 bohdanprog

@bohdanprog I built a simple project to reproduce the issue. It's just importing the react-native-svg lib.

I'm using webpack because I need react-native-web to use the web implementation of react-native-svg.

yarn add @babel/core @babel/preset-env @babel/preset-react babel-loader html-webpack-plugin webpack webpack-cli [email protected] [email protected] -D

src/App.js

import React from "react";
import { SvgXml } from "react-native-svg";
const App = () => {
  return <h1>Title</h1>;
};

export default App;

webpack.config.js

const path = require("path");
const HtmlWebpackPlugin = require("html-webpack-plugin");

module.exports = {
  output: {
    path: path.join(__dirname, "/dist"),
    filename: "bundle.js",
  },
  plugins: [
    new HtmlWebpackPlugin({
      template: "src/index.html",
    }),
  ],
  module: {
    rules: [
      {
        test: /\.(js|jsx)$/,
        exclude: /node_modules/,
        use: {
          loader: "babel-loader",
        },
      },
    ],
  },
  resolve: {
    extensions: ['.web.ts', '.ts', '.web.js', '.js'],
    alias: {
      'react-native$': 'react-native-web'
    }
  }
};

src/index.js

import React from "react";
import ReactDOM from "react-dom";
import App from "./App";

const el = document.getElementById("app");

ReactDOM.render(<App />, el);

src/index.html

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta http-equiv="X-UA-Compatible" content="IE=edge" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title></title>
  </head>
  <body>
    <div id="app"></div>
    <script src="bundle.js"></script>
  </body>
</html>

.babelrc

{
  "presets": ["@babel/preset-env", "@babel/preset-react"]
}

If I try to run this project npm run webpack --mode production, the build fails because the dependency @react-native/assets-registry/registry is missing.

ERROR in ./node_modules/react-native-svg/lib/module/lib/resolveAssetUri.js 3:0-70
Module not found: Error: Can't resolve '@react-native/assets-registry/registry' in '/Users/matheus/cantoo/rn-svg-sample/node_modules/react-native-svg/lib/module/lib'

If I manually add the dependency yarn add @react-native/assets-registry, I get an issue related to webpack not being able to bundle the file ./node_modules/@react-native/assets-registry/registry.js. I think this second issue isn't directly related to react-native-svg, but it impacts the end users.

ERROR in ./node_modules/@react-native/assets-registry/registry.js 13:7
Module parse failed: Unexpected token (13:7)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
| 'use strict';
| 
> export type PackagerAsset = {
|   +__packager_asset: boolean,
|   +fileSystemLocation: string,

MatheusrdSantos avatar Aug 20 '24 14:08 MatheusrdSantos

Hi @MatheusrdSantos, IDK why but your simple repro isn't simple :D I created a repository with a simple example, can you modify that?

bohdanprog avatar Aug 20 '24 15:08 bohdanprog

@bohdanprog I created this pull request: https://github.com/bohdanprog/RNSVG-XML-web/pull/1. I noticed that @react-native/assets-registry/registry was missing because [email protected] requires the react-native version to be ^0.72.3. I was using an older version, so it was my falt.

But the second issue remains.

MatheusrdSantos avatar Aug 20 '24 18:08 MatheusrdSantos

@MatheusrdSantos Hm as I see that issue exists when we use the webpack configuration, but IDK why. Maybe is it a matter of configuration? I guess it's the same issue

bohdanprog avatar Aug 20 '24 19:08 bohdanprog

@bohdanprog yes, it's the same issue.

The webpack issue is a matter of configuration that will impact all react-native-svg users that don't have a proper config to parse flow files. So it's a breaking change for those users.

MatheusrdSantos avatar Aug 20 '24 19:08 MatheusrdSantos

@MatheusrdSantos I added the correct webpack config to the example app, can you check that?

bohdanprog avatar Aug 21 '24 09:08 bohdanprog