gloriasoft / veaury

Use React in Vue3 and Vue3 in React, And as perfect as possible!
MIT License
1.31k stars 83 forks source link

Svg icons won't show in grafana/ui library react components #10

Closed AlexanderBrese closed 2 years ago

AlexanderBrese commented 2 years ago

Hello there I use vue 3, vite, grafana/ui, veaury

first of thank you very much for this library it surely helps a lot. I try to use it with grafana/ui which is made up of react components internally. When I try to use a Button for example and pass an icon string prop it internally processes the svg with react-inline-svg and then uses a dom parser to get the node of the parsed svg text. For some reason it doesn't work for me. I do get the following error: grafik Which is located here: grafik And used first here: grafik Inside grafana_ui.js One of the components in question: grafik The processed svg is correct: data:image/svg+xml,%3csvg id='Layer_1' data-name='Layer 1' xmlns='http://www.w3.org/2000/svg' viewBox='0 0 24 24'%3e%3cpath d='M12%2c6a.99974.99974%2c0%2c0%2c0-1%2c1v4H9a1%2c1%2c0%2c0%2c0%2c0%2c2h3a.99974.99974%2c0%2c0%2c0%2c1-1V7A.99974.99974%2c0%2c0%2c0%2c12%2c6Zm0-4A10%2c10%2c0%2c1%2c0%2c22%2c12%2c10.01146%2c10.01146%2c0%2c0%2c0%2c12%2c2Zm0%2c18a8%2c8%2c0%2c1%2c1%2c8-8A8.00917%2c8.00917%2c0%2c0%2c1%2c12%2c20Z'/%3e%3c/svg%3e

It seemed to work in vuera with vue 2.

devilwjp commented 2 years ago

@AlexanderBrese This doesn't seem to be a veaury problem. When I create a new react project, import @grafana/ui, and use IconButton, this error occurs

And I also found this issue, which is very similar to this problem

I think @grafana/ui doesn't support vite well enough, it is recommended that you use webpack to build vue3

devilwjp commented 2 years ago

@AlexanderBrese I tried using @grafana/ui@canary and everything works fine!
But the canary version (version 9) is unofficial, which can prove that the problem is not caused by veaury. @grafana/ui only supports up to version 17.0.2 of react, so don't install the latest version of react

This is a sample project, you can download and install it for debugging, you can view the configuration of vite.config.js, because @grafana/ui does not support vite well, you need to manually make some fixes. vue3-vite-grafana.zip

AlexanderBrese commented 2 years ago

@devilwjp Thank you Sir. It works now. Another thing I've noticed is that I do get the following error when I try to run pnpm build:

error during build: Error: 'default' is not exported by node_modules/.pnpm/react@17.0.2/node_modules/react/index.js, imported by node_modules/.pnpm/veaury@1.0.5/node_modules/veaury/dist/veaury.esm.js at error (/home/alex/WebstormProjects/frontend/node_modules/.pnpm/rollup@2.74.1/node_modules/rollup/dist/shared/rollup.js:198:30) at Module.error (/home/alex/WebstormProjects/frontend/node_modules/.pnpm/rollup@2.74.1/node_modules/rollup/dist/shared/rollup.js:12555:16) at Module.traceVariable (/home/alex/WebstormProjects/frontend/node_modules/.pnpm/rollup@2.74.1/node_modules/rollup/dist/shared/rollup.js:12914:29) at ModuleScope.findVariable (/home/alex/WebstormProjects/frontend/node_modules/.pnpm/rollup@2.74.1/node_modules/rollup/dist/shared/rollup.js:11543:39) at FunctionScope.findVariable (/home/alex/WebstormProjects/frontend/node_modules/.pnpm/rollup@2.74.1/node_modules/rollup/dist/shared/rollup.js:6517:38) at ChildScope.findVariable (/home/alex/WebstormProjects/frontend/node_modules/.pnpm/rollup@2.74.1/node_modules/rollup/dist/shared/rollup.js:6517:38) at FunctionScope.findVariable (/home/alex/WebstormProjects/frontend/node_modules/.pnpm/rollup@2.74.1/node_modules/rollup/dist/shared/rollup.js:6517:38) at ChildScope.findVariable (/home/alex/WebstormProjects/frontend/node_modules/.pnpm/rollup@2.74.1/node_modules/rollup/dist/shared/rollup.js:6517:38) at MemberExpression.bind (/home/alex/WebstormProjects/frontend/node_modules/.pnpm/rollup@2.74.1/node_modules/rollup/dist/shared/rollup.js:8736:49) at CallExpression.bind (/home/alex/WebstormProjects/frontend/node_modules/.pnpm/rollup@2.74.1/node_modules/rollup/dist/shared/rollup.js:5361:23)

Any idea?

Tried https://vitejs.dev/guide/dep-pre-bundling.html#monorepos-and-linked-dependencies, didn't work. I've installed react@17.0.2 and react-dom@17.0.2 as well. My vite.config.ts:

import { defineConfig } from 'vite'
import vue from '@vitejs/plugin-vue'
import { VitePWA } from 'vite-plugin-pwa'
import { URL } from 'url'
import VueSvg from '@juit/vite-plugin-vue-svg'
import { visualizer } from 'rollup-plugin-visualizer'
import Components from 'unplugin-vue-components/vite'
import checker from 'vite-plugin-checker'
import { NodeGlobalsPolyfillPlugin } from '@esbuild-plugins/node-globals-polyfill'
import { NodeModulesPolyfillPlugin } from '@esbuild-plugins/node-modules-polyfill'
import rollupNodePolyFill from 'rollup-plugin-node-polyfills'
import * as path from 'path'

// https://vitejs.dev/config/
export default defineConfig({
  base: '/',
  root: '',
  build: {
    commonjsOptions: {
      include: [/react/, /vue3-openlayers/, /vue-grid-layout/, /timezones/],
    },
    target: 'es2022',
    rollupOptions: {
      external: ['@grafana/data', '@grafana/ui'],
      plugins: [
        rollupNodePolyFill(),
        visualizer({
          filename: new URL('./dist/bundle.html', import.meta.url).pathname,
          open: false,
          gzipSize: true,
        }),
      ],
    },
  },
  optimizeDeps: {
    include: ['react', 'vue3-openlayers', 'vue-grid-layout', 'timezones'],
    esbuildOptions: {
      target: 'es2022',
      define: {
        global: 'globalThis',
      },
      // Enable esbuild polyfill plugins
      plugins: [
        NodeGlobalsPolyfillPlugin({
          process: true,
          buffer: true,
        }),
        NodeModulesPolyfillPlugin(),
      ],
    },
  },
  define: {
    'process.env': process.env,
    'process.stderr': {},
  },
  resolve: {
    alias: {
      '@/': new URL('./src/', import.meta.url).pathname,
      '@grafana/aws-sdk': path.resolve(
        __dirname,
        './node_modules/@grafana/aws-sdk/index.js'
      ),
      util: 'rollup-plugin-node-polyfills/polyfills/util',
      sys: 'util',
      events: 'rollup-plugin-node-polyfills/polyfills/events',
      stream: 'rollup-plugin-node-polyfills/polyfills/stream',
      path: 'rollup-plugin-node-polyfills/polyfills/path',
      querystring: 'rollup-plugin-node-polyfills/polyfills/qs',
      punycode: 'rollup-plugin-node-polyfills/polyfills/punycode',
      url: 'rollup-plugin-node-polyfills/polyfills/url',
      string_decoder: 'rollup-plugin-node-polyfills/polyfills/string-decoder',
      http: 'rollup-plugin-node-polyfills/polyfills/http',
      https: 'rollup-plugin-node-polyfills/polyfills/http',
      os: 'rollup-plugin-node-polyfills/polyfills/os',
      assert: 'rollup-plugin-node-polyfills/polyfills/assert',
      constants: 'rollup-plugin-node-polyfills/polyfills/constants',
      _stream_duplex:
        'rollup-plugin-node-polyfills/polyfills/readable-stream/duplex',
      _stream_passthrough:
        'rollup-plugin-node-polyfills/polyfills/readable-stream/passthrough',
      _stream_readable:
        'rollup-plugin-node-polyfills/polyfills/readable-stream/readable',
      _stream_writable:
        'rollup-plugin-node-polyfills/polyfills/readable-stream/writable',
      _stream_transform:
        'rollup-plugin-node-polyfills/polyfills/readable-stream/transform',
      timers: 'rollup-plugin-node-polyfills/polyfills/timers',
      console: 'rollup-plugin-node-polyfills/polyfills/console',
      vm: 'rollup-plugin-node-polyfills/polyfills/vm',
      zlib: 'rollup-plugin-node-polyfills/polyfills/zlib',
      tty: 'rollup-plugin-node-polyfills/polyfills/tty',
      domain: 'rollup-plugin-node-polyfills/polyfills/domain',
    },
  },
  plugins: [
    {
      name: '',
      transform(code) {
        return code.replace(
          /(["'])process\.env\.NODE_ENV/g,
          '$1process\u200b.env\u200b.NODE_ENV'
        )
      },
    },
    rollupNodePolyFill(),
    Components({
      dts: true,
    }),
    checker({ typescript: true }),
    VueSvg(),
    vue(),
    VitePWA({
      mode: 'development',
      includeAssets: ['favicon.ico', 'robots.txt', 'apple-touch-icon.png'],
      base: '/',
      strategies: 'injectManifest',
      filename: 'sw.ts',
      manifest: {
        name: 'Progressive Sailing',
        display: 'standalone',
        start_url: '/',
        theme_color: '#42b983',
        background_color: '#42b983',
        short_name: 'PS',
        icons: [
          {
            src: 'android-chrome-192x192.png',
            sizes: '192x192',
            type: 'image/png',
          },
          {
            src: '/android-chrome-512x512.png',
            sizes: '512x512',
            type: 'image/png',
          },
          {
            src: 'android-chrome-maskable-512x512.png',
            sizes: '512x512',
            type: 'image/png',
            purpose: 'any maskable',
          },
        ],
      },
    }),
  ],
})
devilwjp commented 2 years ago

@AlexanderBrese The problem you mentioned above should be that vite encountered a problem when analyzing commonjs. React is currently a pure CommonJS package that only supports the following usages:

import * as React from 'react'
import { Suspense } from 'react'

But in veaury, I use non-standard usage:

import React from 'react'

This non-standard usage is that webpack, tsc and other tools have done special treatment, uniformly packaged in the form of { default: ... }, and added the default field.
I will fix this in a future version of veaury.
So how do you fix this error now? I think you can add defaultIsModuleExports: false in build.commonjsOptions in vite.config.js.

// The vite.config.js you provided
// .......
export default defineConfig({
 //......
  build: {
    commonjsOptions: {
      include: [/react/, /vue3-openlayers/, /vue-grid-layout/, /timezones/],
      defaultIsModuleExports: false
    },
    // ......
  },
// .......

Try it again, hope that solves the problem!!!

AlexanderBrese commented 2 years ago

https://github.com/rollup/plugins/issues/1111 Seems like defaultIsModuleExports is not in vite as of now.

devilwjp commented 2 years ago

@AlexanderBrese This issue is not a problem to be solved by rollup, but what vite should do, try it!

devilwjp commented 2 years ago

But interestingly, the example project I provided doesn't build with this error

AlexanderBrese commented 2 years ago

Thank you very much it worked like a charm.

But interestingly, the example project I provided doesn't build with this error

That might be because I'm using pnpm but I'm not sure though.

devilwjp commented 2 years ago

You can temporarily manually modify the usage of import React and reactDOM from veaury.esm.js in node_modules

AlexanderBrese commented 2 years ago

You can temporarily manually modify the usage of import React and reactDOM from veaury.esm.js in node_modules

Thanks I'll try that.

Have you ever stumbled upon Po.propTypes is undefined? - I've seen in your example that you have proptypes as a dependency.

devilwjp commented 2 years ago

Many packages depend on prop-types.
image

AlexanderBrese commented 2 years ago

Hmm strange why do I get this error when I try to access the view with grafana/ui in it in the built version...

devilwjp commented 2 years ago

You can run npm ls prop-type to see if all prop-type versions are the same

AlexanderBrese commented 2 years ago

You can run npm ls prop-type to see if all prop-type versions are the same

Ok thats what I thought. I'm using pnpm and with pnpm the list is empty. But in the pnpm-lock.json there are a lot of entries for prop-types. I've tried to install the dependency as well.

devilwjp commented 2 years ago

Can you try adding prop-type to package.json, then delete node_modules and pnpm-lock.json, and try to reinstall

AlexanderBrese commented 2 years ago

I just tried that and also imported PropTypes but it didn't work. I'm importing the view dynamically btw (from vue-router).

devilwjp commented 2 years ago

If you directly import react components dynamically, you need to use lazyReactInVue

AlexanderBrese commented 2 years ago

Ok I just tried to directly load the view and the error persists.

devilwjp commented 2 years ago

@AlexanderBrese Will loading the view into the sample project I provided display fine? If it works fine, then compare the differences in the configuration of these two project

AlexanderBrese commented 2 years ago

Thanks I will definitely try this. Sir I can not thank you enough for all your help.

AlexanderBrese commented 2 years ago

I'm not sure what the issue is the error message that i get in the browser doesn't really help. I've tried your example and just ran npm run build and it didn't work (Uncaught TypeError: ui.PropTypes is undefined). Could you please check it too?

devilwjp commented 2 years ago

@AlexanderBrese OK, can you provide an example for me to debug? It would be better to add your example to the example project I provided.

AlexanderBrese commented 2 years ago

You should be able to receive the error when you run "npm run build" on the example project you gave me. I had the same issue there in build mode.

devilwjp commented 2 years ago

@AlexanderBrese Is the following error message?
image

AlexanderBrese commented 2 years ago

Yes I think you get the propTypes error message when you run it on firefox but I'm not sure. I got different ones on chrome and firefox.

devilwjp commented 2 years ago

This is still a commonjs problem, I need to configure it.

AlexanderBrese commented 2 years ago

I tried setting defaultIsModuleExports: false. Didn't work.

devilwjp commented 2 years ago

@AlexanderBrese Wow!!!,I found the cause of the problem!!! @grafana/ui/index.production.js require ./ReactMonacoEditor.production.js by causing a circular dependency.
And index.production.js dynamically requires ReactMonacoEditor.production.js through Promise, so you need to specify this dynamic require target in vite.config.js.

image

So you need to use dynamic require, and install the latest version of @rollup/plugin-commonjs. In the include of commonjsOptions , there is no need to add any path.

commonjsOptions: {
  // '@grafana/ui' require './ReactMonacoEditor.production.js' by causing a circular dependency
  // So you need to use dynamic require
  dynamicRequireTargets : [
    'node_modules/@grafana/ui/ReactMonacoEditor.production.js' ,
  ]
}

My project already builds fine and works smoothly! image

AlexanderBrese commented 2 years ago

Sir, thank you for all you’ve done. I owe you big-time. I will try that out.