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

Image: new babel/webpack loader #237

Closed
necolas opened this issue Oct 23, 2016 · 23 comments
Closed

Image: new babel/webpack loader #237

necolas opened this issue Oct 23, 2016 · 23 comments
Labels
enhancement Requires extension or creation of new React Native API needs: help

Comments

@necolas
Copy link
Owner

necolas commented Oct 23, 2016

We need a new webpack image loader that can generate RN source objects from assets on the file system:

require('./image.png');
// => { uri: '...', width: 40, height: 40 }

It should also support RN's syntax for resolution-specific images:

...by adding extra data to the object so that Image can choose the most appropriate asset:

require('./image.png');
// => { uri: '...', uri@2x: '...', uri@3x: '...', width: 40, height: 40 }
@necolas necolas added enhancement Requires extension or creation of new React Native API needs: help labels Oct 23, 2016
@MoOx
Copy link
Contributor

MoOx commented Oct 27, 2016

Going to try this https://www.npmjs.com/package/react-native-image-loader
(can't find the repo, the only reference I found is https://github.com/willpiers/react-native-image-loader from mjohnston/react-native-webpack-server#6 but leads to a 404)

@MoOx
Copy link
Contributor

MoOx commented Oct 27, 2016

Ok false alarm, this module is just 5 lines of code and totally don't work as expected :(
Will try to do one then.

@MoOx
Copy link
Contributor

MoOx commented Oct 27, 2016

I did this pretty quickly and it seems to work for now

const fs = require("fs")

const ImageFile = require("image-file")

module.exports = function(content) {
  const cleanPublicPath = content
    .replace("module.exports = ", "")
    .replace(/;$/, "")

  const path = this.resourcePath
  const image = new ImageFile(fs.readFileSync(path).buffer)

  return `module.exports = {
    uri: ${ cleanPublicPath },
    width: "${ image.width }",
    height: "${ image.height }"
  }`
}

Usage in webpack config:

        {
          test: /\.(jpg|png|gif)$/,
          loader: "./src/loaders/react-native-web-image-loader.js!file-loader",
        },

@necolas
Copy link
Owner Author

necolas commented Oct 28, 2016

Nice start. How do we get webpack to extract any other resolution-specific images?

@MoOx
Copy link
Contributor

MoOx commented Oct 28, 2016

Not handled yet in my case, but I guess the next step would be:

const fs = require("fs")

const ImageFile = require("image-file")

module.exports = function(content) {

  //
  // @todo instead of using original file path, we should just reuse some
  // file-loader logic directly to support hash for file paths etc
  //

  const cleanPublicPath = content
    .replace("module.exports = ", "")
    .replace(/;$/, "")

  const absoluteFile = this.resourcePath
  const image = new ImageFile(fs.readFileSync(absoluteFile).buffer)

  const path = require("path")

  const absoluteData = path.parse(absoluteFile)
  const base = absoluteData.dir + absoluteData.name

  const fileData = path.parse(cleanPublicPath)
  const cleanBase = fileData.dir + fileData.name

  let cleanPublicPath2x
  let cleanPublicPath3x
  try {
    const x2 = fs.readFileSync(base + "@2x" + absoluteData.ext)
    cleanPublicPath2x = cleanBase + "@2x" + fileData.ext
    this.emitFile(cleanPublicPath2x, x2)
  }
  catch (e) {
    // do nothing, just ignore @2x file
  }
  try {
    const x3 = fs.readFileSync(base + "@3x" + absoluteData.ext)
    cleanPublicPath3x = cleanBase + "@3x" + fileData.ext
    this.emitFile(cleanPublicPath3x, x3)
  }
  catch (e) {
    // do nothing, just ignore @3x file
  }

  return `module.exports = {
    uri: ${ cleanPublicPath },
    ${ cleanPublicPath2x ? `uri@2x: ${ cleanPublicPath2x }` : "" },
    ${ cleanPublicPath3x ? `uri@3x: ${ cleanPublicPath3x }` : "" },
    width: "${ image.width }",
    height: "${ image.height }"
  }`
}

This code is currently pretty dumb and does not take in consideration the fact that path emitted by file-loader can be hashes like 0dcbbaa701328a3c262cfd45869e351f.png and will emit [email protected] for 2x version (which is stupid, and should just be another hash, see comment at the top of this snippet)

@giuseppeg
Copy link
Contributor

You may want to fork file-loader which internally uses loader-utils and takes care of hashing etc. Additional resolution info could be passed along with other configuration options like so: res: [1, 2, 3] maybe.

@sjmueller
Copy link

sjmueller commented Nov 24, 2016

Will this also support absolute source uri in production? Scenario: using create-react-app to build the site, and all static assets get hosted on a cdn while the core app is deployed to heroku. Right now a build results in relative paths like

/static/media/icon.79190308.png

when we need paths like

https://img.s3-us-east-1.amazonaws.com/static/media/icon.79190308.png

EDIT: Found this issue on create-react-app that discusses the above use case: facebook/create-react-app#936 . We'll have to see if the corresponding PR addresses it.

@EnoahNetzach
Copy link

EnoahNetzach commented Nov 25, 2016

@sjmueller FYI the related PR is this one facebook/create-react-app#937, but I can't see how that will help this issue.
Probably you can just use an environment variable and call it a day, if my understanding of your needs are not wrong.

@peter-jozsa
Copy link

peter-jozsa commented Jan 16, 2017

I have created a webpack loader, which takes care of appropriate scaling usage too(based on device pixel ratio). Check it out, and feel free to contribute. This is my first npm package so please if something is wrong you can tell me :)
https://www.npmjs.com/package/react-native-web-image-loader

I have plans to release a webpack plugin too which is a resolver. For me, image suffixes were pain in the *ss since some module required its asset (./assets/back.png) but even the 1x scaling had a suffix(./assets/[email protected]) and webpack didn't seem to find it.

@necolas
Copy link
Owner Author

necolas commented Jan 17, 2017

Cool. This library already has modules that provide the pixel ratio so the wrapper module the plugin injects seems redundant. Interested in moving development of the plugin into this repo?

@peter-jozsa
Copy link

I did not notice that you had already take care of pixel ratios. I think we could move it here since an out-of-box solution is better than installing separate packages. How would you like to impement it here?

@peter-jozsa
Copy link

well I did not get any response from necolas, and currently I'm too busy with office work.

@jwickens
Copy link

jwickens commented May 1, 2017

I've used @psychoo118 's package and it works well. Since using react-native-web requires editing your webpack config already why don't we just update the documentation to refer to the new webpack loader? I know @necolas mentioned that there is some redundancy in getting the pixel ratio but its only a couple of lines.

@appden
Copy link
Contributor

appden commented May 27, 2017

FYI, Haul includes an assetLoader and AssetResolver that mostly works with react-native-web. There are caveats, like it expects an AssetRegistry module which react-native-web doesn't have, but you can fake with your own implementation through some webpack trickery.

This issue somewhat relates to callstack/haul#132, where they are discussing making Haul compatible with react-native-web.

@necolas
Copy link
Owner Author

necolas commented May 28, 2017

I wonder if some of webpack-specific parts of Haul (like the image loader with scale support) could be published as npm packages for use outside of Haul. But if we can get Haul working with RNW too that would be great.

Relevant parts of React Native to work into RNW:

necolas referenced this issue in fmoo/react-native-web Jul 11, 2017
@necolas
Copy link
Owner Author

necolas commented Jul 12, 2017

I think this functionality is possibly best introduced as a babel-plugin, that way it could be used with Metro, Haul, or Webpack rather than being tied to webpack. Not sure what Metro does to support this in React Native

@necolas necolas changed the title Image: new webpack loader Image: new babel/webpack loader Jul 12, 2017
Repository owner deleted a comment from DaKaZ Aug 11, 2017
Repository owner deleted a comment from dusave Aug 11, 2017
@AlexBrasileiro
Copy link

@psychoo118 you save our lives :D!

@peter-jozsa
Copy link

glad to hear that :)

@RWOverdijk
Copy link

Hei! How does this work? It doesn't seem automatic yet and this issue is 2 years old so I'm curious what I can do to get them crisp images 😎

@EvanBacon
Copy link
Contributor

Personally I think this feature is better suited to a universal <picture /> element or props like srcset="..." sizes="..." being added to the upstream Image component. Adding new bundler functionality to every bundler seems like a far more complex task and makes it harder to keep RNW bundler agnostic.

@RWOverdijk
Copy link

I understand the reasoning, I get it, but I disagree. It sucks that I can't load images that contain an @ in the name. Now I renamed all images and I always have to load the correct one myself, even on mobile.

I think that at least having a clear guide on this would be much better than that.

@RWOverdijk
Copy link

Hello, it's me again. I still don't have a satisfying approach for this. What I am doing now is renaming all images to be image.png, image_2x.png, image_3x.png and then using the PixelRation.get() to use the correct image. The problem with this is that the expo service worker downloads all the images for all densities and it makes things quite heavy.

With this issue still being open, I am curious what the next steps should be. I don't mind doing the work, I just need some pointers

@necolas
Copy link
Owner Author

necolas commented Aug 7, 2020

Closing this because no one has really worked on the bundler side for 4 years and the teams working on RN packagers should figure this out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Requires extension or creation of new React Native API needs: help
Projects
None yet
Development

No branches or pull requests