airbnb / react-sketchapp

render React components to Sketch ⚛️💎
http://airbnb.io/react-sketchapp/
MIT License
14.95k stars 824 forks source link

Svg import broken #515

Closed macintoshhelper closed 4 years ago

macintoshhelper commented 4 years ago

I am...

------------------------------------------------------------------------------------------------- Reporting a bug or issue

Expected behavior:

image

Observed behavior:

> basic-svg@1.0.0 postinstall /Users/<redacted>/basic-svg
> npm run build && skpm-link

> basic-svg@1.0.0 build /Users/<redacted>/basic-svg
> skpm-build

[1/2] 🖨  Copied src/manifest.json in 3ms
error Error while building ./my-command.js
./node_modules/node-gyp-build/index.js
Module not found: Error: Can't resolve 'fs' in '/Users/<redacted>/basic-svg/node_modules/node-gyp-build'
resolve 'fs' in '/Users/<redacted>/basic-svg/node_modules/node-gyp-build'
  Parsed request is a module
  using description file: /Users/<redacted>/basic-svg/node_modules/node-gyp-build/package.json (relative path: .)
    Field 'browser' doesn't contain a valid alias configuration
  after using description file: /Users/<redacted>/basic-svg/node_modules/node-gyp-build/package.json (relative path: .)
    resolve as module

How to reproduce:

curl https://codeload.github.com/airbnb/react-sketchapp/tar.gz/master | tar -xz --strip=2 react-sketchapp-master/examples/basic-svg
cd basic-svg

Sketch version:

63.1

Please attach ~screenshots, a zip file of your project, and~ or a link to your github project

https://github.com/airbnb/react-sketchapp/tree/master/examples/basic-svg

macintoshhelper commented 4 years ago

I believe this is caused by https://github.com/airbnb/react-sketchapp/blob/9f4b38cdfb7848b32ff9c1968b145dabd72739a9/src/components/index.ts#L8

Proposed fix:

import Svg from './Svg';
export { Svg };

Fixed in 78c317019391eaa6ea0505c09bdb2e7a2c1ccf76, but may be best to revert and apply in a new atomic PR.

I believe the import * as Svg is not actually needed, as this is already handled by applying static references at https://github.com/airbnb/react-sketchapp/blob/9f4b38cdfb7848b32ff9c1968b145dabd72739a9/src/components/Svg/Svg.tsx#L49 . On the other hand, I'm surprised that this was working though with commonjs, but not with the es2015 module, there must be some difference in the commonjs interop polyfills compared to ES.

mathieudutour commented 4 years ago

Wait I’m confused, the error is completely unrelated to the svg import.

The import * as is on purpose (feature from a previous PR). And I’m not sure how this is going to fix the error

macintoshhelper commented 4 years ago

Wait I’m confused, the error is completely unrelated to the svg import.

The import * as is on purpose (feature from a previous PR). And I’m not sure how this is going to fix the error

Ah, sorry, that error seems to be caused by something else. I managed to get rid of it by aliasing react-sketchapp to lib/module (not sure how to reproduce this a proper fix though, quite a strange/cryptic error), then was getting this:

image

And with doing this in the lib/module/components/index.js file:

export { Document } from './Document';
export { Page } from './Page';
export { Artboard } from './Artboard';
export { Image } from './Image';
export { RedBox } from './RedBox';
export { View } from './View';
export { Text } from './Text';
- import * as Svg from './Svg';
+ import Svg from './Svg';
export { Svg };

it renders fine

I can only find import * as Svg since the platform refactor. import Svg, { Circle, ... } from 'react-sketchapp/lib/components/Svg' should still work I think.

edit Ah, is the import * as Svg for allowing destructuring?

import { Svg } from 'react-sketchapp';

const { G, Path } = Svg;

Here's the full console log (seems to be caused by src/platformBridges/macos.ts, strange, since it works fine in basic-setup:

[1/2] 🖨  Copied src/manifest.json in 3ms
error Error while building ./my-command.js
/Users/<redacted>/react-sketchapp/node_modules/node-gyp-build/index.js
Module not found: Error: Can't resolve 'fs' in '/Users/<redacted>/react-sketchapp/node_modules/node-gyp-build'
resolve 'fs' in '/Users/<redacted>/react-sketchapp/node_modules/node-gyp-build'
  Parsed request is a module
  using description file: /Users/<redacted>/react-sketchapp/node_modules/node-gyp-build/package.json (relative path: .)
    Field 'browser' doesn't contain a valid alias configuration
  after using description file: /Users/<redacted>/react-sketchapp/node_modules/node-gyp-build/package.json (relative path: .)
    resolve as module
      /Users/<redacted>/react-sketchapp/node_modules/node-gyp-build/node_modules doesn't exist or is not a directory
      /Users/<redacted>/react-sketchapp/node_modules/node_modules doesn't exist or is not a directory
      /Users/<redacted>/node_modules doesn't exist or is not a directory
      /Users/<redacted>/node_modules doesn't exist or is not a directory
      /Users/<user>/Documents/node_modules doesn't exist or is not a directory
      /Users/<user>/node_modules doesn't exist or is not a directory
      /Users/node_modules doesn't exist or is not a directory
      /node_modules doesn't exist or is not a directory
      looking for modules in /Users/<redacted>/react-sketchapp/node_modules
        using description file: /Users/<redacted>/react-sketchapp/package.json (relative path: ./node_modules)
          Field 'browser' doesn't contain a valid alias configuration
        after using description file: /Users/<redacted>/react-sketchapp/package.json (relative path: ./node_modules)
          using description file: /Users/<redacted>/react-sketchapp/package.json (relative path: ./node_modules/fs)
            no extension
              Field 'browser' doesn't contain a valid alias configuration
              /Users/<redacted>/react-sketchapp/node_modules/fs doesn't exist
            .sketch.js
              Field 'browser' doesn't contain a valid alias configuration
              /Users/<redacted>/react-sketchapp/node_modules/fs.sketch.js doesn't exist
            .js
              Field 'browser' doesn't contain a valid alias configuration
              /Users/<redacted>/react-sketchapp/node_modules/fs.js doesn't exist
            as directory
              /Users/<redacted>/react-sketchapp/node_modules/fs doesn't exist
      looking for modules in /Users/<redacted>/github/node_modules
        using description file: /Users/<redacted>/github/package.json (relative path: ./node_modules)
          Field 'browser' doesn't contain a valid alias configuration
        after using description file: /Users/<redacted>/github/package.json (relative path: ./node_modules)
          using description file: /Users/<redacted>/github/package.json (relative path: ./node_modules/fs)
            no extension
              Field 'browser' doesn't contain a valid alias configuration
              /Users/<redacted>/github/node_modules/fs doesn't exist
            .sketch.js
              Field 'browser' doesn't contain a valid alias configuration
              /Users/<redacted>/github/node_modules/fs.sketch.js doesn't exist
            .js
              Field 'browser' doesn't contain a valid alias configuration
              /Users/<redacted>/github/node_modules/fs.js doesn't exist
            as directory
              /Users/<redacted>/github/node_modules/fs doesn't exist
      looking for modules in /Users/<redacted>/react-sketchapp/examples/basic-svg/node_modules/@skpm/builder/node_modules
        using description file: /Users/<redacted>/react-sketchapp/examples/basic-svg/node_modules/@skpm/builder/package.json (relative path: ./node_modules)
          Field 'browser' doesn't contain a valid alias configuration
        after using description file: /Users/<redacted>/react-sketchapp/examples/basic-svg/node_modules/@skpm/builder/package.json (relative path: ./node_modules)
          using description file: /Users/<redacted>/react-sketchapp/examples/basic-svg/node_modules/@skpm/builder/package.json (relative path: ./node_modules/fs)
            no extension
              Field 'browser' doesn't contain a valid alias configuration
              /Users/<redacted>/react-sketchapp/examples/basic-svg/node_modules/@skpm/builder/node_modules/fs doesn't exist
            .sketch.js
              Field 'browser' doesn't contain a valid alias configuration
              /Users/<redacted>/react-sketchapp/examples/basic-svg/node_modules/@skpm/builder/node_modules/fs.sketch.js doesn't exist
            .js
              Field 'browser' doesn't contain a valid alias configuration
              /Users/<redacted>/react-sketchapp/examples/basic-svg/node_modules/@skpm/builder/node_modules/fs.js doesn't exist
            as directory
              /Users/<redacted>/react-sketchapp/examples/basic-svg/node_modules/@skpm/builder/node_modules/fs doesn't exist
[/Users/<redacted>/react-sketchapp/node_modules/node-gyp-build/node_modules]
[/Users/<redacted>/react-sketchapp/node_modules/node_modules]
[/Users/<redacted>/node_modules]
[/Users/<redacted>/node_modules]
[/Users/<user>/Documents/node_modules]
[/Users/<user>/node_modules]
[/Users/node_modules]
[/node_modules]
[/Users/<redacted>/react-sketchapp/node_modules/fs]
[/Users/<redacted>/github/node_modules/fs]
[/Users/<redacted>/react-sketchapp/node_modules/fs.sketch.js]
[/Users/<redacted>/github/node_modules/fs.sketch.js]
[/Users/<redacted>/react-sketchapp/node_modules/fs.js]
[/Users/<redacted>/github/node_modules/fs.js]
[/Users/<redacted>/react-sketchapp/node_modules/fs]
[/Users/<redacted>/github/node_modules/fs]
[/Users/<redacted>/react-sketchapp/examples/basic-svg/node_modules/@skpm/builder/node_modules/fs]
[/Users/<redacted>/react-sketchapp/examples/basic-svg/node_modules/@skpm/builder/node_modules/fs.sketch.js]
[/Users/<redacted>/react-sketchapp/examples/basic-svg/node_modules/@skpm/builder/node_modules/fs.js]
[/Users/<redacted>/react-sketchapp/examples/basic-svg/node_modules/@skpm/builder/node_modules/fs]
 @ /Users/<redacted>/react-sketchapp/node_modules/node-gyp-build/index.js 1:9-22
 @ /Users/<redacted>/react-sketchapp/node_modules/node-sketch-bridge/index.js
 @ /Users/<redacted>/react-sketchapp/lib/module/platformBridges/macos.js
 @ /Users/<redacted>/react-sketchapp/lib/module/index.js
 @ ./src/my-command.js
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! basic-svg@1.0.0 build: `skpm-build`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the basic-svg@1.0.0 build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/<user>/.npm/_logs/2020-05-10T08_26_54_644Z-debug.log
mathieudutour commented 4 years ago

So the issue is that it's trying to bundle lib/module/platformBridges/macos.js but shouldn't (should bundle the sketch bridge instead)

mathieudutour commented 4 years ago

Oh I think it's because of

"module": "lib/module/index.js",
"sketch": "lib/module/index.sketch.js",

skpm prioritize the module entry point instead of the sketch one

macintoshhelper commented 4 years ago

Ah, great catch, adding .sketch.js to the module entry seems to solve that issue.

Am still getting Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: object. after that though.

mathieudutour commented 4 years ago

I'll fix it

macintoshhelper commented 4 years ago

Great, thanks 🙂. I've undone the import Svg from patch in #516 , ~that was fixing it for me before, but isn't fixing it now anyway it seems 😕~, anyway, that'll break destructuring from Svg I think.

macintoshhelper commented 4 years ago

Seems the import Svg from fix (removing * as) resolves the Element type is invalid error.

I just tested destructuring const { G, Path } = Svg; with it too, and that seems to still work. I can't think of any drawbacks to removing the import * as bit. react-native-svg compatible API should remain.

macintoshhelper commented 4 years ago

Thanks a lot, ignore my last message. Works now with 3.2.5 🎉.

Looks like using export * from './components'; instead of export * from './index.common'; in src/index.ts helped 🤔, not sure though.

Esorakouki commented 4 years ago

@mathieudutour @macintoshhelper I think the latest version does't work completely. My code does't work when I update react-sketchapp to 3.2.4 or 3.2.5. image

The Svg default export was broken.

image

I need change my code this way to get the default export(Svg itself).

macintoshhelper commented 4 years ago

The Svg default export was broken.

Hi, I've been running into this too (from running the basic-svg example). I'm using this code as a manual patch (inside node_modules):

export { Document } from './Document';
export { Page } from './Page';
export { Artboard } from './Artboard';
export { Image } from './Image';
export { RedBox } from './RedBox';
export { View } from './View';
export { Text } from './Text';
- import * as Svg from './Svg';
+ import Svg from './Svg';
export { Svg };

import * as seems to be breaking the default export.

const { default: SvgContainer } = Svg; feels pretty unsafe to me, you should be able to use <Svg> instead