Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transformer for asset does not work #176

Closed
kruyvanna opened this issue May 30, 2018 · 29 comments
Closed

Transformer for asset does not work #176

kruyvanna opened this issue May 30, 2018 · 29 comments

Comments

@kruyvanna
Copy link

kruyvanna commented May 30, 2018

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
I am trying to create a transformer that process .svg files.
Currently it only feed .js files to the transformer not .svg files

If the current behavior is a bug, please provide the steps to reproduce and a minimal repository on GitHub that we can yarn install and yarn test.

  1. yarn install
  2. react-native run-ios / react-native run android
  3. check metro packager log for .svg file (console.log from transformer. see svg_transformer)

Minimal repository:
https://github.com/kruyvanna/metro-asset-transformer-test

What is the expected behavior?
transformer.transform get called with .svg file

Please provide your exact Metro configuration and mention your Metro, node, yarn/npm version and operating system.
i have rn-cli.config.js with the following content

module.exports = {  
  assetTransforms: true,
  getPlatforms: () => ["ios", "android"],
  getTransformModulePath() {  	        
    	return require.resolve('./svg_transformer')    
  },
  getAssetExts() {
    return ["svg"];
  }
};

node: v8.9.1
yarn: 1.7.0
npm: 5.6.0
react-native-cli: 2.0.1
react-native: 0.55.4

@rafeca
Copy link
Contributor

rafeca commented May 30, 2018

Thanks for reporting the issue @kruyvanna !

I've cloned your test repo and found a couple of things that need to be changed to be able to run the SVG transformer correctly. I've prepared a commit to show you these changes: rafeca/metro-asset-transformer-test@b5b8034

EDIT: all the text below is not relevant anymore

Unfortunately this is not enough, and a fix needs to be done in the react-native repository. The main issue is that react-native is considering getAssetExts() as additional asset extensions to include in the default ones, and is basically concatenating the user-defined asset extensions with the default ones (this is on of the places where this is done: https://github.com/facebook/react-native/blob/master/local-cli/server/runServer.js#L180). This prevents your user-defined config to remove svg from the asset extensions.

The best way to fix this would be to do a breaking change and change the meaning of the assetExts configuration param, so instead of being additional extensions it should be the actual extensions to be used.

Ideally, to make the API nicer, the getAssetExts() should receive the default extensions as a parameter, so any user would be able to add/remove extensions easily. e.g:

//getAssetExts(defaultExts) {
//  // remove "svg" and add "jpegg"
//  return defaultExts.filter(ext !== 'svg').concat('jpegg');
//}

To do so, these are steps that need to be done:

  1. Change each of the calls to getAssetExts() in both React Native and Metro repository to pass the default assets (the ones in metro/src/defaults) as a param. This should be done in:

  2. Change the default value of getAssetExts() in metro repository, to just return the default ones instead of an empty array (the ones received as a param). This should be done in:

  3. Remove the concatenation of the custom asset extensions with the default ones in metro repository. This should be done in:

  4. Remove the concatenation of assetExts in the react-native repository. This should be done in:

    (This is going to be the big breaking change)

Unfortunately, I don't think I'm going to have bandwidth to do this any time soon, but if you want to collaborate I'll be happy to review the PRs (this is a nice project to start collaborating in metro/React Native 😃).

@kruyvanna
Copy link
Author

@rafeca Thanks for the detail instruction.
I will give it a try

@kristerkari
Copy link

kristerkari commented Aug 2, 2018

I created my own SVG transformer based on the same idea. It uses .svgx file extension instead of .svg, because loading .svg files through the transformer is not supported due to the reasons that @rafeca mentioned above.

I'm planning on switching to using .svg as soon as it's supported in React Native/Metro.

https://github.com/kristerkari/react-native-svg-transformer

@rafeca
Copy link
Contributor

rafeca commented Aug 2, 2018

I just want to give an update since many things happened since I wrote that message in May: we've recently rewritten the Metro configuration loading logic (you can read about the new config here) so the limitation regarding the default assets does not exist anymore.

With latest Metro you can define the asset extensions that you want without having to care about the default ones, so please ignore most of my previous message 😄

@rafeca
Copy link
Contributor

rafeca commented Aug 2, 2018

Closing this issue since with the new Metro API it should be solved

@rafeca rafeca closed this as completed Aug 2, 2018
@kristerkari
Copy link

@rafeca I saw some Metro configuration changes in React Native's repo. Do you know if those are going to fix the issue on React Native's side?

@rafeca
Copy link
Contributor

rafeca commented Aug 3, 2018

Yes, they fix it

@kristerkari
Copy link

kristerkari commented Aug 24, 2018

I'm having problems to get the new Metro configuration to work.

I have been trying out the 0.57.0-rc.2 version of React Native, which also includes the new Metro config. I'm using a working 0.56.0 project as the base.

I've read the code changes from React Native's side and the updated docs for Metro, and based on that my updated rn-cli.config.js for transforming SVG files looks like this:

module.exports = {
  transformModulePath: require.resolve("react-native-svg-transformer"),
  resolver: {
    sourceExts: ["svg"]
  }
};

The new config however just makes Metro throw an error:

error: bundling failed: ReferenceError: SHA-1 for file /Users/kristerkari/Git/SVGDemo/index.js is not computed
    at DependencyGraph.getSha1 (/Users/kristerkari/Git/SVGDemo/node_modules/metro/src/node-haste/DependencyGraph.js:236:119)
    at /Users/kristerkari/Git/SVGDemo/node_modules/metro/src/Bundler.js:203:56
    at Generator.next (<anonymous>)
    at step (/Users/kristerkari/Git/SVGDemo/node_modules/metro/src/Bundler.js:11:657)
    at /Users/kristerkari/Git/SVGDemo/node_modules/metro/src/Bundler.js:11:817
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)

@rafeca do you have any ideas why that might happen?

@rafeca
Copy link
Contributor

rafeca commented Sep 11, 2018

@kristerkari : this is usually caused by watchFolders not containing the folder of that file. watchFolders should be automatically configured to include the root of your app, so this error is weird... Can you try updating to the latest RC and check if this still happens?

@kristerkari
Copy link

@rafeca it's still happening with rc4

@kristerkari
Copy link

@rafeca Here are the changes and the branch if you want to have a look at it:
kristerkari/react-native-svg-example#1

@rafeca
Copy link
Contributor

rafeca commented Sep 11, 2018

Oh I see now, the issue comes from the fact that making sourceExts: ["svg"] removes the default source extensions, so the bundler cannot find the JS files.

To fix this, you can get the default source extensions and just append svg from your Metro config:

const {getDefaultConfig} = require('metro');

module.exports = (async () => {
  const {resolver: {sourceExts}} = await getDefaultConfig();

  return {
    transformer: {
      babelTransformerPath: require.resolve('react-native-svg-transformer'),
    },
    resolver: {
      sourceExts: [...sourceExts, 'svg'],
    },
  };
})();

I acknowledge that this is not very nice to type... @CompuIves could we make it easier to extend the config object?

@kristerkari
Copy link

Would it be possible to have some config property that would just add the file extension to the default ones instead of replacing them?

Would be great to keep the config as simple as my previous example:

module.exports = {
  transformModulePath: require.resolve("react-native-svg-transformer"),
  resolver: {
    sourceExts: ["svg"]
  }
};

@kristerkari
Copy link

I managed to get the .svg images to load in React Native!

The config to do that is very verbose because I also need to modify assetExts no to include svg file extension:

const { getDefaultConfig } = require("metro-config");

module.exports = (async () => {
  const {
    resolver: { sourceExts, assetExts }
  } = await getDefaultConfig();

  return {
    transformer: {
      babelTransformerPath: require.resolve("react-native-svg-transformer")
    },
    resolver: {
      assetExts: assetExts.filter(ext => ext !== "svg"),
      sourceExts: [...sourceExts, "svg"]
    }
  };
})();

That's pretty difficult to understand for someone who is not familiar with Metro at all.

@Samsinite
Copy link

Hi Guys! I'm getting a bundling failed: ReferenceError: SHA-1 for file /Users/samueljclopton/workspace/wildland/cherish-frontend/index.js is not computed error as well, but I'm not sure how to track down the root cause of this. Do you guys have any suggestions for how you determined that the SVG file transforming were the root cause of the issues experienced here?

@isbirkan
Copy link

isbirkan commented Oct 19, 2018

Hi guys!

I am so glad I stumbled upon this issue. It managed to put me on the right track. I am trying to add .pb and .txt assets to my project for a tensorflow integration. The above configuration works, but it behaves strange in my case.

If I only add a single optional extension it works as expected, but if I try to add 2 it throws an error.

For example, the code below throws an error:

const { getDefaultConfig } = require('metro-config');

module.exports = (async () => {
  const {
    resolver: { assetExts }
  } = await getDefaultConfig();

  return {
    resolver: {
      assetExts: [...assetExts, 'pb', 'txt']
    }
  };
})();

And the error is:
error: bundling failed: SyntaxError: Unexpected end of JSON input at JSON.parse (<anonymous>) at FileStore.get (*path*\node_modules\metro-cache\src\stores\FileStore.js:26:19) at *path*\node_modules\metro-cache\src\Cache.js:76:40 at Generator.next (<anonymous>) at step (*path*\node_modules\metro-cache\src\Cache.js:18:30) at *path*\node_modules\metro-cache\src\Cache.js:37:14 at new Promise (<anonymous>) at *path*\node_modules\metro-cache\src\Cache.js:15:12 at Cache.get (*path*\node_modules\metro-cache\src\Cache.js:102:7) at *path*\node_modules\metro\src\DeltaBundler\Transformer.js:166:34

If I remove one of the extensions:

assetExts: [...assetExts, 'pb']

the bundle is created, but I need both extensions in order to have the application working.

Maybe someone has/had a similar problem and can manage to help me.

@sometimescasey
Copy link

@isbirkan I've been working on a similar project and the following works for me in metro.config.js:

module.exports = {
  resolver: {
    assetExts: ['pb', 'txt', 'jpg']
  }
}

I followed the documentation at https://facebook.github.io/metro/docs/en/configuration. Hope this helps.

@isbirkan
Copy link

isbirkan commented Nov 5, 2018

Hi @sometimescasey!

Thank you very much! It works! I have tried this in the past, but I think I omitted 'jpg' and since I know it was a default asset extension I just tried to extend the config as others have done before. But this is ok and since it works it is all I need.

@punksta
Copy link

punksta commented Jan 18, 2019

I have similar error when I enable ram bundle on android

 bundling failed: ReferenceError: SHA-1 for file /home/_/app/node_modules/metro/src/lib/polyfills/require.js is not computed

"metro-react-native-babel-preset": "^0.50.0" also on 0.51.1
"react-native": "0.57.8"
android

@stylehuan
Copy link

+1

@idudinov
Copy link

idudinov commented Jul 24, 2019

Hey guys!
I'm building an app on Expo SDK 33, and using react-native-svg-transformer with metro.config.js updated according to this issue and official documentation of the package.
Today (literally) Metro started to skip svg files in my custom transformer (custom because I'm using the solution from here: https://github.com/kristerkari/react-native-css-modules/blob/master/docs/multiple-transformers.md#step-4-configure-transformer), so svg files got unresolved during bundling with the following error:

Unable to resolve module `../assets/images/welcome-screen-persona.svg` from `<...>/src/screens/WelcomeScreen.tsx`: The module `../assets/images/welcome-screen-persona.svg` could not be found from `<...>/src/screens/WelcomeScreen.tsx`. Indeed, none of these files exist:
  * `<...>/src/assets/images/welcome-screen-persona.svg(.native||.ios.expo.js|.native.expo.js|.expo.js|.ios.expo.ts|.native.expo.ts|.expo.ts|.ios.expo.tsx|.native.expo.tsx|.expo.tsx|.ios.expo.json|.native.expo.json|.expo.json|.ios.js|.native.js|.js|.ios.json|.native.json|.json|.ios.ts|.native.ts|.ts|.ios.tsx|.native.tsx|.tsx)`
  * `<...>/src/assets/images/welcome-screen-persona.svg/index(.native||.ios.expo.js|.native.expo.js|.expo.js|.ios.expo.ts|.native.expo.ts|.expo.ts|.ios.expo.tsx|.native.expo.tsx|.expo.tsx|.ios.expo.json|.native.expo.json|.expo.json|.ios.js|.native.js|.js|.ios.json|.native.json|.json|.ios.ts|.native.ts|.ts|.ios.tsx|.native.tsx|.tsx)`

According to these logs, looks like svg is not treated as extension as all, and Metro does try to add source extensions to full path of it, so cannot resolve the file.

Here's my full metrol.config.js:

const { resolve } = require('path');
const { getDefaultConfig } = require('metro-config');

module.exports = (async () => {
    const {
        resolver: { sourceExts, assetExts }
    } = await getDefaultConfig();

    return {
        projectRoot: resolve(__dirname),
        watchFolders: [
            resolve(__dirname, '../common'),
        ],
        transformer: {
            // that's my custom transformer in which I won't receive svg files
            babelTransformerPath: require.resolve('./metro.transformer.js'),
        },
        resolver: {
            // I've made sure extensions got updated
            assetExts: assetExts.filter(ext => ext !== "svg"),
            sourceExts: [...sourceExts, "svg"],
            // below is for using external common folder with shared code
            extraNodeModules: {
                common: resolve(__dirname, '../common'),
                mobx: resolve(__dirname, 'node_modules/mobx'),
                tslib: resolve(__dirname, 'node_modules/tslib'),
                '@babel/runtime': resolve(__dirname, 'node_modules/@babel/runtime'),
                'firebase': resolve(__dirname, 'node_modules/firebase'),
            },
        },
    };
})();

Please give me an idea what could go wrong! Thanks in advance!

@kristerkari
Copy link

@idudinov Are you using the newest version of React Native? It seems that something has changed, so that the transformers are not working, but I have not had time yet to look at the problem.

@kristerkari
Copy link

Oh sorry I just noticed that you wrote that you are using Expo 33

@idudinov
Copy link

idudinov commented Jul 24, 2019

Just for reference, Expo 33 uses React Native 0.59.8.
I've just checked the last version of my codebase (2 days old) when I was able to build my app, and it doesn't work now! That makes my crazy. Will try to find out what else could change on my side since that happy time when everything worked just great.

@idudinov
Copy link

Also I've tried to replace svg to svgx, and just added it to sourceExts, but no luck – Metro still fails to resolve the file.

One thing I think of could change during these days – I've updated XCode to 10.3. Will try to check on macs with previous version of it.

@idudinov
Copy link

Phew! Downgrading XCode did not help, but downgrading Expo CLI did. Replaced 3.0.x with 2.21.2 and the problem has gone. Spent only a day to work around this! 😂

@fdagostino
Copy link

Phew! Downgrading XCode did not help, but downgrading Expo CLI did. Replaced 3.0.x with 2.21.2 and the problem has gone. Spent only a day to work around this! 😂

Can confirm that doing the downgrading got the thing working.
Any lead on how to get this working on latest version of expo-cli?

@kristerkari
Copy link

@idudinov @fdagostino the docs for react-native-svg-transformer have been updated to include the correct config for expo-cli 3.x https://github.com/kristerkari/react-native-svg-transformer#for-react-native-v057-or-newer--expo-sdk-v3100-or-newer

@fdagostino
Copy link

@idudinov @fdagostino the docs for react-native-svg-transformer have been updated to include the correct config for expo-cli 3.x https://github.com/kristerkari/react-native-svg-transformer#for-react-native-v057-or-newer--expo-sdk-v3100-or-newer

Thanks @kristerkari, worked fine!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants