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

"react-native link" silently fails to link non-font assets #16446

Closed
andriichernenko opened this issue Oct 18, 2017 · 13 comments
Closed

"react-native link" silently fails to link non-font assets #16446

andriichernenko opened this issue Oct 18, 2017 · 13 comments
Assignees
Labels
Stale There has been a lack of activity on this issue and it may be closed soon.

Comments

@andriichernenko
Copy link

andriichernenko commented Oct 18, 2017

Is this a bug report?

Yes

Have you read the Contributing Guidelines?

Yes

Environment

Environment:
OS: macOS Sierra 10.12.6
Node: 7.10.0
Yarn: 0.27.5
npm: 4.2.0
Watchman: 4.7.0
Xcode: Xcode 9.0.1 Build version 9A1004
Android Studio: 2.3 AI-162.4069837

Packages: (wanted => installed)
react: 16.0.0-alpha.12 => 16.0.0-alpha.12
react-native: 0.48.4 => 0.48.4

Steps to Reproduce

  1. Put font and non-font assets into a project directory (e.g. ./assets/fonts/file.ttf ./assets/text/file.txt).
  2. Add the following section to package.json:
"rnpm": {
    "assets": [
      "assets/text",
      "assets/fonts"
    ]
  },
  1. Run react-native link.

Expected Behavior

All asset files are copied to iOS and Android projects. If that's not possible, an error or warning is reported in console. If for some reason that's not possible either, this behavior is documented.

Actual Behavior

file.ttf is copied as expected. file.txt is not copied, console reports that linking was successful, there are no errors or warnings.

Reproducible Demo

https://github.com/andriichernenko/rnpm-non-font-assets-repro

Here's the code in /node_modules/react-native/local-cli/link/ios/copyAssets.js that copies the assets:

/**
 * Copies each file from an array of assets provided to targetPath directory
 *
 * For now, the only types of files that are handled are:
 * - Fonts (otf, ttf) - copied to targetPath/fonts under original name
 */
module.exports = function copyAssetsAndroid(files, targetPath) {
	const assets = groupFilesByType(files);

	(assets.font || []).forEach(asset =>
		fs.copySync(asset, path.join(targetPath, 'fonts', path.basename(asset)))
	);
};

As the comment says, "the only types of files that are handled are fonts". One would think that other types are handled somewhere else, but that's not the case.

Seems like this has been reported before (see #14468). There's also a related PR: #12047, but it wasn't merged.

Are there any plans to implement handling of resources of all types?

@Kureev
Copy link
Contributor

Kureev commented Oct 19, 2017

Hi @andriichernenko!

This is an expected behavior (so far). I don't see how .txt file can be a font file (as stated in your config file). Can you please describe the goal you're trying to achieve so we can figure out a solution?

@Kureev Kureev self-assigned this Oct 19, 2017
@andriichernenko
Copy link
Author

@Kureev I have a large text file (~500 KB, it's a document with license information for the open-source libraries we are using) which I would like to update automatically and include in the app as an asset. Later in code I want to be able to access this file, read and display its contents without worrying about the platform-specific things.

There are many other file types which developers might need to include as assets (videos, sounds; some people, for example, might want to include an SQLite database), so I was surprised when I found out that react-native link handles only fonts.

The logic which does all the heavy lifting (like adding the files to a group in iOS project and to the Copy Bundle Resources phase) is already in place, I don't think it will be too much work to handle additional file types. On the RN side of things it would be nice to have something like AssetManager on Android, but for now we could use react-native-fs and handle the assets as ordinary files.

@Kureev
Copy link
Contributor

Kureev commented Oct 19, 2017

Oh, don't get me wrong: I have no issues adding new functionality to handle this. For some reason I had a feeling that for iOS it requires additional logic to register a font (not sure if its true, fyi @grabbou).

Based on your explanation, I feel like you want to use react-native link to link additional files to the app package which never was a purpose of the tool. Although, if people feels it this way, I have no objections implementing a "fallback" solution for non-text files.

What do you think, Mike?

@Yaowenjie
Copy link

Yaowenjie commented Oct 19, 2017

I have this problem as well. I'm still confused with the same question as @andriichernenko said: why does react-native link handle only fonts?

In my situation, I have a custom RN library with some license files and other static files (which need to be added into Copy Bundle Resources phase if you want to use it). If I want to use this library in my RN project, I must copy these files and add them to Copy Bundle Resources manually. So... Why not react-native link help us to do this?

@grabbou
Copy link
Contributor

grabbou commented Oct 19, 2017

@andriichernenko we added assets linking functionality to react-native link was to handle fonts specifically. The reason is that setting up custom fonts requires native code to work. You have to add associated font files to your iOS / Android projects as native assets and then, configure them to be discovered as a font.

With non-font files, like txt, I see no reason why you couldn't just require('path/to/a.txt') and profit. I've been doing that with html and other related files w/o any issues so far.

Would you mind telling why the above would not work for you?

@andriichernenko
Copy link
Author

andriichernenko commented Oct 19, 2017

@grabbou require() works for HTML files, as well as .mp3, .wav, .mov, .pdf and some other formats. With .txt files, however, it fails with "Unable to resolve module" error, and I was unable to figure out why. I ended up wrapping my text file in HTML and displaying it in WebView.

Even if require() worked for text files, it returns a number. As a person who had never worked with JS before starting with React Native a few months ago, I have no idea what that means and what to do with it. I could probably look at WebView to see how it handles its source when it's a require(), but... Wouldn't it be better if developer experience was a bit smoother?

Okay, even if react-native link works as intended, the situation when it happily reports that everything was linked successfully while in fact nothing was linked doesn't help at all. I cannot be productive as a developer if I cannot trust my tools.

@grabbou
Copy link
Contributor

grabbou commented Oct 19, 2017

@andriichernenko in regards to this:

require() works for HTML files, as well as .mp3, .wav, .mov, .pdf and some other formats. With .txt files, however, it fails with "Unable to resolve module" error, and I was unable to figure out why. I ended up wrapping my text file in HTML and displaying it in WebView.

Just start your packager this way: react-native start --assetExts txt. The default list of assets doesn't have txt predefined.

Even if require() worked for text files, it returns a number

Returns a number that is a reference to an asset in AssetRegistry - place where assets are stored along with additional information about their scales, location - an implementation detail.

I have no idea what that means and what to do with it

Will debug this in the morning to see how to read that file. I've done it once, but it's been a while.

@andriichernenko
Copy link
Author

andriichernenko commented Oct 19, 2017

The default list of assets doesn't have txt predefined

Is there a chance it will be added?

Returns a number that is a reference to an asset in AssetRegistry - place where assets are stored along with additional information about their scales, location - an implementation detail.

Oh, I see. AssetRegistry module contains function getAssetByID(assetId: number): PackagerAsset, although I was not able to import this module, is it private?

UPD: never mind, require('AssetRegistry') worked despite complaints from Flow. I can get the asset info using getAssetByID. Now I need to figure out how to get its content. Any advice on this?

@grabbou
Copy link
Contributor

grabbou commented Oct 30, 2017

Is there a chance it will be added?

I think yeah, shouldn't be an issue. Just submit PR adding it if you think it's a reasonable default. I think it would be cool to be able to import txt file into a string, but that's against Node behavior AFAIK.

Now I need to figure out how to get its content

I don't have time to dig into it, but look how Image module handles it - it works the same way. I belive given that id of an asset, you should be able to parse it.

Let me know if you need any further assistance, happy to help. As I said, I don't have enough time to dig into it myself.

@andriichernenko
Copy link
Author

@grabbou thanks for help, I'll see what I can do.

Just for the reference, in case anyone ends up using require(./file.html), beware of this bug: #16133.

@stale
Copy link

stale bot commented Dec 29, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. If you think this issue should definitely remain open, please let us know why. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Dec 29, 2017
@stale stale bot closed this as completed Jan 5, 2018
@unimonkiez
Copy link

unimonkiez commented Jan 25, 2018

I wrote react-native-asset which solves just that.
It also unlinks old files automatically, which I find pretty helpful.

Currently supports only fonts and sound files, but just finished writing it so might add more in the future.
Added support for any file extension.

@alejandro-menendez
Copy link

It helps me . https://stackoverflow.com/a/49457267/2666902

@facebook facebook locked and limited conversation to collaborators May 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Stale There has been a lack of activity on this issue and it may be closed soon.
Projects
None yet
Development

No branches or pull requests

6 participants