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

Transform static image references into externals #7

Merged
merged 1 commit into from
Apr 19, 2015

Conversation

colinhicks
Copy link
Contributor

When left intact, react-native's static image references, e.g. require('image!fooBar'), are problematic. By default, webpack attempts to resolve a loader called webpack-image-loader (plus many derivative names) and to resolve a module ('fooBar') based on the string after the bang. This yields compile errors, in my experience.

So, seems these references need to be short-circuited. This PR does that by generating corresponding externals:

require('image!fooBar')

// yields
{ uri: 'fooBar', isStatic: true }

I believe this resolves #6 assuming @boyswan and I were seeing the same thing.

@colinhicks colinhicks mentioned this pull request Apr 17, 2015
@mjohnston
Copy link
Owner

Interesting approach. This topic came up here as well: facebook/react-native#395 (comment)

Maybe @amasad can shed some light on whether or not the {uri, isStatic} syntax is likely to stay intact with the upcoming changes in the react packager?

@amasad
Copy link

amasad commented Apr 17, 2015

This should be fine for now, in the future we'll have to change it to contain different information. I'm planning on posting an issue on the react-native repo once we have finalized the details but here is quick run down. Let me know if you have any feedback.

There are a few problems with way we currently manage assets:

  1. All images live in a single namespace. This is a problem for obvious reasons
  2. Images don't live with the code by default. We use assetRoots to tell the packager where the assets should live.
  3. You have to go add them to XCode (and follow what other platforms we want to target in the future)
  4. You can't easily publish images with components to npm and have them just work

So we're looking to replace the current system with the following:

  1. Images can exist alongside the JS code and be required via relative paths (i.e. commonjs resolution). require('./path/to/img.png')
  2. Images can have multiple resolution versions and are expressed via the @Nx notation currently used in iOS. So img.png could actually be [email protected] or img.png or what have you
  3. Images will still compile to static modules but will have different information (url, path, width, height) -- this is not final yet
  4. In development: The client will then request the image from the server with the resolution it needs (the server will give it the closest resolution it could find)
  5. In production: That's not final yet, but we're thinking of generating a zip or tar file including the assets
  6. This is platform agnostic, and can work on iOS, Android and whatever platforms we'll be targeting in the future

What would that mean for this project, is that we change it from {uri, isStatic} to whatever data we decide on sending and then the image requests can be delegated to react-packager (or implemented here, doesn't really matter).

@mjohnston
Copy link
Owner

Ok I'll pull in this change, and we can revisit when react native's image packaging details are finalized.

mjohnston pushed a commit that referenced this pull request Apr 19, 2015
Transform static image references into externals
@mjohnston mjohnston merged commit feeb336 into mjohnston:master Apr 19, 2015
@colinhicks colinhicks deleted the transform-static-image-refs branch April 19, 2015 13:19
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

Successfully merging this pull request may close these issues.

image! require error
3 participants