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

Dynamic imports of static files are not working as expected #533

Closed
randex opened this issue Mar 3, 2020 · 14 comments
Closed

Dynamic imports of static files are not working as expected #533

randex opened this issue Mar 3, 2020 · 14 comments
Labels
kind: support Asking for support with something or a specific use case scope: integration Related to an integration, not necessarily to core (but could influence core)

Comments

@randex
Copy link

randex commented Mar 3, 2020

Current Behavior

I have a folder full of country flags SVGs, and I need to display a specific one. I think that the best solution here is to just dynamically import it into an img.

<Select renderValue={value => <img src={require(`../flags/${value}.svg`)} />}>

I ended up spending whole day trying to figure out how to make it work. Also tried ES6 imports and just setting src to ../flags/${value}.svg (without using any imports), but the storybook keeps failing with an error Module not found: Error: Can't resolve '../flags' in... all the time.

I've also tried to copy the whole flags folder into dist using rollup-plugin-copy with this config:

const copy = require('rollup-plugin-copy');
const rpts2 = require('rollup-plugin-typescript2');

module.exports = {
  rollup (config, options) {
    config.plugins = [
      ...config.plugins,
      copy({
        target: [{ src: 'src/flags/**/*', dest: 'dist/flags' }],
      }),
    ];

    config.plugins = config.plugins.map(plugin => {
      if (plugin && plugin.name === 'rpt2') {
        return rpts2({
          // properties that I added for demonstration purposes
          clean: true,
          objectHashIgnoreUnknownHack: true,
          // properties in the current internal implementation of tsdx
          typescript: require('typescript'),
          cacheRoot: `./.rts2_cache_${options.format}`,
          tsconfig: options.tsconfig,
          tsconfigDefaults: {
            compilerOptions: {
              sourceMap: true,
              declaration: true,
              jsx: 'react',
            },
          },
          tsconfigOverride: {
            compilerOptions: {
              target: 'esnext',
            },
          },
        });
      }

      return plugin;
    });

    return config;
  },
};

But the folder just won't appear in the dist.

I'm not experienced with Rollup unfortunately, so maybe there's someone who can help me use static files like these in my package?

Expected behavior

Correct handling of dynamic imports of static files.

Suggested solution(s)

N/A

Additional context

Your environment

Software Version(s)
TSDX 0.12.3
TypeScript 3.8.3
Browser Microsoft Edge 80 (Chromium)
npm/Yarn npm v6.13.7
Node 12.13.0
Operating System macOS 10.15.4 Beta (19E242d)
@agilgur5
Copy link
Collaborator

agilgur5 commented Mar 4, 2020

I'm confused, is this for Storybook stories or for distribution as a library? Because those have different configs.

On Storybook, there was a big change with fixes in #435. I'm not sure if that's related however. It still hasn't been released though... see #512

For Rollup images -- there is this HOWTO: #379 (comment) . I'm not totally sure how that interacts with dynamic imports but it wasn't clear if you'd tried that.

@randex
Copy link
Author

randex commented Mar 4, 2020 via email

@agilgur5
Copy link
Collaborator

agilgur5 commented Mar 4, 2020

I didn’t try to npm run build yet, was just building my component right in the Storybook

Uhuh... well if you haven't used npm run build, then you haven't run Rollup/TSDX what-so-ever... You've just been running Storybook, which has its own configuration. tsdx.config.js has nothing to do with Storybook.

The thing is, I can’t see “flags” folder in my “dist” after compilation no matter what I try.

Storybook doesn't produce a ./dist/ folder at all

Sounds like you're not using things correctly and mixing two different tools up.

@randex
Copy link
Author

randex commented Mar 4, 2020

Hmm, I've been thinking that npm run start runs Rollup and uses tsdx.config.js... I've been running npm run start and npm run storybook in two Terminals at the same time, just like it's said in readme.

@agilgur5
Copy link
Collaborator

agilgur5 commented Mar 4, 2020

Ah, ok gotcha. start is (mostly) the same as build, it just runs a watcher.

Seems like the docs might be wrong there as the way the stories are configured, they are imported from src/ and not dist/ (at least since #435 , the version before that was buggy so I am not sure how it worked)

@randex
Copy link
Author

randex commented Mar 4, 2020

Oh, I see.
I've managed to hook it up with the example, so now I run npm start from the root of the project to watch and compile the package, and npm start from the example folder to test it, I don't use the Storybook now.

I've also fixed my problem with rollup-plugin-copy not copying my flags folder and now I have a pretty simple folder structure with the flags folder being copied to the root of dist. I've tried that HOWTO again, and I've also tried to use rollup-plugin-svg to fix my problem, but it doesn't seem to work still. But there's a thing that I've noticed:

When I require an SVG using a static string

export default function () {
  return <img src={require("./flags/us.svg")} />
}

It shows the flag without any problem. But when I use a template string

export default function () {
  const locale = 'us'
  return <img src={require(`./flags/${locale}.svg`)} />
}

It crashes with error: example.f69400ca.js:37 Uncaught Error: Cannot find module './flags/us.svg'
I don't really want to import all the flags, so is there a way to fix this error? There is something else I don't understand about dynamic imports and I really want to understand it.

@lpolito
Copy link
Contributor

lpolito commented Mar 4, 2020

Dynamic imports are not statically analyzable (as in, the bundler doesn't know to include dynamically imported files). As far as I know no modern bundler can support them.

You have to either include all of the flags, or just the flags you know will be used.

@randex
Copy link
Author

randex commented Mar 4, 2020

Sorry, I don't mean to argue and stuff, I haven't been doing much configuration lately (just using create-react-app for everything) and I don't make libraries too often, so I trust you in this, but how did they manage to do it here then? They use nwb, which is based on webpack that's using url-loader, and it works perfectly, putting a nice little path into the img's src attribute.

@agilgur5
Copy link
Collaborator

agilgur5 commented Mar 4, 2020

@lpolito thanks for chiming in. I've dealt with this before but didn't really remember the caveats. Your statement could use some modification though:

Dynamic imports are statically analyzable, that's how code-splitting works in both Rollup and Webpack after all.
The key difference, as @randex noticed, is the inclusion of string interpolation, which makes it much harder to statically analyze. There is an older Rollup issue on this in fact: rollup/rollup#2463

@randex that's an interesting example, I am curious how that simple config works. The Rollup issue points to virtual modules/custom resolvers as a potential solution but I'm not sure if there's a simple way to do it.

@agilgur5
Copy link
Collaborator

agilgur5 commented Mar 4, 2020

@randex actually I think that example is far simpler than we think and has nothing to do with bundlers -- it just includes the entire flags/ directory in its package.json.files.

It also has this issue that you'll end up bundling all flags: ekwonye-richard/react-flags-select#22

@randex
Copy link
Author

randex commented Mar 5, 2020

Thank you very much for your research and that useful link to Rollup issue. I've tried to use a virtual module here but I'm stuck again with a problem.
index.tsx is very simple now, just to test out virtual modules:

import test from 'testmodule'

export default function () {
  return test
}

Now testmodule is what should be resolved by Rollup. I have this config now:

module.exports = {
  rollup (config) {
    config.plugins.unshift({
      name: 'plugin-test-module',
      resolveId (id) {
        console.log('resolveId', id);

        if (id === 'testmodule') {
          return id;
        }

        return null;
      },
      load (id) {
        if (id === 'testmodule') {
          return 'export default "Test is successful"';
        }

        return null;
      },
    });

    return config;
  },
};

So what should happen is I just need to see "Test is successful" in the browser.
Now here comes the weird part: npm run build fails with error Cannot find module 'testmodule'. I've put console.log into resolveId() to see what's happening, and looks like it never receives testmodule in its id. I replaced unshift with just straight assignment to config.plugins (so it removes other ones) and it successfully compiled, although I understand that this is bad, so it's not a solution. I've read Rollup's docs and it seems like some other plugin added by TSDX like node-resolve may be trying to resolve the import instead of my plugin, but I can't find a way to stop that :(

@agilgur5
Copy link
Collaborator

agilgur5 commented Mar 6, 2020

Sorry this is a general Rollup support question and so is getting further and further out-of-scope. I think using rollup-plugin-copy would likely be much simpler than trying to use virtual modules.

I'm not sure why unshift isn't working for you. If you're putting it at the beginning, it should be run before node-resolve. I don't know why it's not working.

@randex
Copy link
Author

randex commented Mar 6, 2020

I understand. Thank you very much for all your help. I’ll post a solution here when I find it for the sake of completeness.

@agilgur5 agilgur5 added scope: integration Related to an integration, not necessarily to core (but could influence core) kind: support Asking for support with something or a specific use case labels Mar 9, 2020
@agilgur5 agilgur5 closed this as completed Mar 9, 2020
@hnipps
Copy link

hnipps commented Jun 30, 2020

Dynamic imports are not statically analyzable (as in, the bundler doesn't know to include dynamically imported files). As far as I know no modern bundler can support them.

You have to either include all of the flags, or just the flags you know will be used.

@lpolito Just want to point out that Webpack supports partially dynamic imports, e.g. import(`./some/dir/${fileName}.js`)

https://webpack.js.org/api/module-methods/#dynamic-expressions-in-import

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: support Asking for support with something or a specific use case scope: integration Related to an integration, not necessarily to core (but could influence core)
Projects
None yet
Development

No branches or pull requests

4 participants