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

RFC: Raw file imports without webpack-specific syntax #2961

Closed
gaearon opened this Issue Aug 16, 2017 · 24 comments

Comments

Projects
None yet
@gaearon
Member

gaearon commented Aug 16, 2017

Update: here's my latest thinking on this: #2961 (comment). This comment is old.


Currently we can't unambiguously tell how you want to treat a random file so we fall back to the simplest behavior for everything except JS and CSS. For example if you import file.md or file.svg we give you a URL to it because we don't know for sure what you need to do with it.

This works for many cases but can be annoying when you want to treat a file like data. For example, #1388 describes a use case for importing SVG as a React component, and #595 describes a use case for loading Markdown files.

We don't want to use Webpack loader syntax for this because it is Webpack-specific and other tools don't understand it. However I now realized we might not need to.

What if we determined this by file extension instead? For example,

import docs from './docs.raw.md';
import iconSVG from './icon.raw.svg';

Sure, this means you have to encode how the file is used into the file itself, and also need to have control over it. For example this wouldn't work if you wanted to import SVG from an npm package. But on the other hand this doesn't work anyway, so at least this doesn't make it worse, but provides an escape hatch for cases when you need that extra option.

This approach is consistent with other cases where we might look at filenames. For example there is an ongoing proposal to use .module.css for opting into CSS modules.

What do you think?


Update: here's my latest thinking on this: #2961 (comment). This comment is old.

@Timer

This comment has been minimized.

Collaborator

Timer commented Aug 16, 2017

Related: #1944

tl;dr: fs.readFileSync()

This .raw.ext format may be better if we adopt the same convention for other things.

@msikma

This comment has been minimized.

msikma commented Aug 16, 2017

I just wanted to briefly restate my comment from #595, regarding loading in Markdown files. While it's true it would be easier to just use import and get the file's contents, the current situation isn't all that bad. You can still load files asynchronously like this:

// Returns the URL as a string, e.g. '/static/media/changelog.1fb9caf0.md'.
// It will be present in the build directory.
import myMarkdownFile from '../../../changelog.md';

fetch(myMarkdownFile)
  .then(response => response.text())
  .then(text => {
    // Logs a string of Markdown content.
    // Now you could use e.g. <rexxars/react-markdown> to render it.
    console.log(text);
  });

For this use case, this works just fine I think. This also allows you to select on the fly whether you want a Response's text() or blob(). I'm not sure; if we add support in the Webpack configuration for loading arbitrary files, could that be problematic for text files that need to be loaded as text rather than raw bytes? Or vice versa?

@ArmorDarks

This comment has been minimized.

ArmorDarks commented Aug 17, 2017

I'm concerned with fetch idea, because it will hit performance badly. Imagine, you have 1 file, which used across different instances 100 times. In some cases it will result in 100 standalone fetches, each hitting disk operation.

I'm not sure how thing with import will work, but ideally it have to load file only once, thus minimizing disk operations.

@gaearon

This comment has been minimized.

Member

gaearon commented Aug 17, 2017

This is not a problem because you can put a simple JS cache in front of fetch() and keep results in an object to avoid fetching the same thing twice. But in general it is a valid use case to want to have access to some data immediately without even a single extra fetch.

@msikma

This comment has been minimized.

msikma commented Aug 17, 2017

I agree on that. The main reason why I mention it is because if this proposal doesn't pan out for whatever reason this might be something good to document, because I think some people were confused about how to accomplish this currently. (Although this far from ideal, especially for files small enough to easily be part of the bundle.)

We don't want to use Webpack loader syntax for this because it is Webpack-specific and other tools don't understand it. However I now realized we might not need to.

It would be kind of nice if we could just use e.g. require('url!file.md') but I see your point. It would probably break all other tools like ESLint. On the other hand it's also a little strange to have to name files file.raw.md to be able to import them as data. Then you'd get questions like "why is this named .raw.md instead of just .md?" and you have to point to CRA-specific documentation. Not that it's such a bad thing, but one of the nice things about CRA is you don't really need to know anything about this tool specifically—all you need to know is React.

I'm not against this proposal, though—just raising questions to try and observe the issue from as many sides as possible. 🙂

@ArmorDarks

This comment has been minimized.

ArmorDarks commented Aug 17, 2017

I'm not very familiar with Webpack itself, but maybe some ideas from SystemJS plugin-raw can be taken.

Syntax for import looks quite elegant and straight-understandable:

import docs from './docs.md!raw';

And ESLint is happy with that. I never checked, though, does ! loaders meta-information is a part of import specs, or just a specific to SystemJS thing.

I think the main issue with import docs from './docs.raw.md'; is that it makes assumptions in a wrong direction. Just think of that — when you add a file to repository, do you think in advance "yeah, I will import that thing as raw, I need to add special extension for that"?

This extension becomes even more obscure, when you look at the file outside of CRA environment (which isn't unusual situation). Why that file have that strange .raw part? Can I remove it?

Example with module.css isn't that relevant here, since module.css isn't useful on its own, it is specific subset of CSS Modules, of CSS, while when it comes to other files, they are useful standalone units, which can be used in other environments, outside of CRA, for different things. Marking them in advance with .raw doesn't make much sense.

For instance, I need project README.md. Should I now rename it to README.raw.md just because I need it raw? After all, it will break Github expectations (didn't check, though, maybe it still would be able to pick up the file).

I think it would be at least somehow less confusing, if something like that would be possible to do:

import docs from './docs.md.raw';

Where docs.md is real filename, and .raw is a specific meta-marker. But it makes it quite close to !raw.

@dgautsch

This comment has been minimized.

dgautsch commented Aug 18, 2017

I like the systemjs option but webpack has something similar with this loader https://github.com/webpack-contrib/raw-loader. That way we aren't adding new dependencies.

@jimniels

This comment has been minimized.

jimniels commented Aug 21, 2017

It would be nice to be able to load a file in different ways without regard to the extension (I know CRA doesn't officially support building libraries, but that's a good example case).

For example, if I want to load a .js file as raw data or as a .js file, I could do either without having the extension dictate how it is loaded. Going the route of .js.raw means I have to have multiple files if I want to use them in different ways (a MyThing.js and MyThing.js.jaw), whereas something more like .js!raw doesn't dictate that. One file, multiple uses.

Again, I realize CRA doesn't officially support doing libraries, but if raw loading is going to be supported, why not kill two birds with one stone and support it in a way that in the future, if you wanted, this kind js-library-way-of-doing-things would be supported.

@callil

This comment has been minimized.

callil commented Aug 22, 2017

Just want to add that the lack of something like import md from 'raw-loader!./file.md'; in create-react-app has been the only cause of my needing to npm eject so something that handles this would be great. Async is just too much of a performance hit.

I don't like the idea of changing the file name because it makes any workflow that's outside of create-react-app a pain. For instance working with .md or .svg in tools that handle them would be made that much more difficult.

I'm with @jimniels & @ArmorDarks

@Clement-Bresson

This comment has been minimized.

Clement-Bresson commented Sep 14, 2017

Same as @callil, I needed npm eject for this reason only.
And I agree it would be better not to change files extension. I like the solution @ArmorDarks suggests with !raw.

@ben-davis

This comment has been minimized.

ben-davis commented Oct 3, 2017

Unless I'm not seeing an issue with this approach, there's no need to eject to use raw-loader:

const webpackMarkdownLoader = require.context(
  '!raw-loader!./markdownFiles',
  false,
  /\.md$/,
);

const markdownFiles = webpackMarkdownLoader
    .keys()
    .map(filename => webpackMarkdownLoader(filename));
@sontek

This comment has been minimized.

sontek commented Nov 13, 2017

@ben-davis That probably will miss loading images from markdown, so it won't properly be loaded/rendered, correct?

@nhuttrung

This comment has been minimized.

nhuttrung commented Nov 14, 2017

@sontek A possible solution is putting .md and image files in public folder.

App.js:

import React, { Component } from 'react';
import ReactMarkdown from 'react-markdown';

const webpackRequireContext = require.context(
  '!raw-loader!../public',
  false,
  /\.md$/,
)

// Convert to Map
const files = webpackRequireContext.keys().reduce((map, fileName) => {
  const markdown = webpackRequireContext(fileName)
  // remove the leading './'
  if (fileName.startsWith('./')){
    fileName = fileName.substr(2)
  }

  return map.set(fileName, markdown);
}, new Map())

class App extends Component {
  render() {
    return (
      <ReactMarkdown source={files.get('markdown-with-image.md')} />
    );
  }
}
@GAumala

This comment has been minimized.

Contributor

GAumala commented Dec 28, 2017

Using webpack specific code in the react app code kinda defeats the purpose of using create-react-app. Accepting that kind of solution would imply having to add a section to the User guide that explains how to user webpackRequireContext to load text files. This contradicts the idea that you should not have to know anything about the underlying tools in order to use create-react-app.

The systemJS plugin-raw approch and the fs.readFileSync a la browserify are good solutions, but personally I prefer using .raw prefix before extension solely because of consistency with the CSS modules proposal.

@gaearon

This comment has been minimized.

Member

gaearon commented Jan 8, 2018

If we had some default behavior for MD, which would it be? Would it return HTML? Raw Markdown?

@gaearon

This comment has been minimized.

Member

gaearon commented Jan 8, 2018

For SVG we sort of settled on #1388 (comment).

@GAumala

This comment has been minimized.

Contributor

GAumala commented Jan 8, 2018

Personally I would prefer raw markdown as the default behavior so that I can directly feed it to some tool that converts it to React components with whatever style I want, but I'm curious about other use cases.

@gaearon

This comment has been minimized.

Member

gaearon commented Jan 8, 2018

Raw markdown seems pretty inefficient since now you have to ship a markdown parser in your app.

@jonathaningram

This comment has been minimized.

jonathaningram commented Jan 9, 2018

@gaearon for my current need I'm also going to say raw markdown.

Correct me if I'm wrong, but if you return HTML does that mean that CRA does the parsing itself and thus CRA ships the parser?

If HTML is returned, then how could one's app decide whether a <h1 /> should actually be rendered as a <h1 class="whatever-h43h2" />? E.g. if that h1 was generated by a styled-component. See https://github.com/rexxars/react-markdown#options renderers prop.

@Superpencil

This comment has been minimized.

Superpencil commented Jan 9, 2018

  • create-react-app hides webpack, if you can configure webpack you don't need create-react-app I think
  • css and svg seem to 'just work' from the perspective of a create-react-app user
  • it might be good to have something that imports single Markdown files that contain blog posts, or recipes, or whatever, and renders them as dumb components

To this end, I'd propose to treat Markdown files as if they were Components:

import MyPost from './MyPost.md'

const Something = (props) => (
  <div>
    <MyPost />
  </div>
)

This approach would perhaps allow for using props inside of the markdown... (red alert...)

Or the same way as SVG's;

import mypost from './mypost.md'

const Something = (props) => (
  <div>
    {mypost}
  </div>
)

I think it would fit in with the logic already established by the SVG's and images; you import something, then you put it somewhere manually.

I only use create-react-app, I don't do very fancy stuff, and I hope this might give a more 'dumb react user' perspective. I do know how to use webpack, kinda, I just don't need to. Would something like this make sense?

@gaearon

This comment has been minimized.

Member

gaearon commented Jan 9, 2018

What if we allowed you to choose what you want?

import { raw } from './recipe.md';
import { html } from './recipe.md';

@gaearon gaearon changed the title from RFC: Raw file imports with .raw prefix before extension to RFC: Raw file imports without webpack-specific syntax Jan 9, 2018

@gaearon gaearon added this to the 2.0.0 milestone Jan 9, 2018

@ndelangen

This comment has been minimized.

Contributor

ndelangen commented Jan 9, 2018

@gaearon That looks like a syntax I think most people can get behind, it's very clear what the intend is.

To achieve this we could write a multi-loader-markdown to that would expose the predefined output types?

The default export could be what most users are familiar with now, so this:

import post from './post.md';

would continue to work.

@gaearon

This comment has been minimized.

Member

gaearon commented Jan 9, 2018

Closing in favor of #3722 I just filed.

@gaearon gaearon closed this Jan 9, 2018

@Evaneaston

This comment has been minimized.

Evaneaston commented Jan 9, 2018

I agree with @ndelangen.

@Timer Timer removed this from the 2.0.0 milestone Jan 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment