How to include/exclude when I do dynamic import using template string?

[al3xnag]

In ember-auto-import 2, is it possible to control which files to include/exclude when I do dynamic import using template string?

await import(`my-library/lang/${lang}`);

For example, some libraries support lots of languages, but I only need a few. I don't want all of them to be copied into dist/assets/.

[ef4]

This is a little tricky because we're trying to stick to standards-defined Javascript as much as possible, and the standard doesn't have a way to cover this intention. But yeah, it would be good to accept a hint here.

Webpack has its magic comments that cover this. We could decide to propagate such comments forward to webpack, effectively allowing people to opt in to using webpack-specific hinting in their app.

Under embroider you could already just use a webpack comment in the above code and it would work. That doesn't work under ember-auto-import because webpack doesn't look directly at your code -- it looks at a summarize of all your imports produced by the ember-auto-import analyzer.

[Panman82]

Does this issue get to the fact that auto-import seems to "lazy load" everything within the my-library/lang/* folder?? Is everything actually included in the build output? I noticed the lazy flag in an error when it tried to lazy load a node-only file within the same folder as what I was actually trying to import.

[ef4]

It generally is lazy (although if the targeted module is really tiny webpack might decide to inline it instead of putting it in a new lazy chunk).

Since it's lazy, you won't ever load a bunch of unused code into your build using this pattern. It shouldn't have runtime impact. It may have build-speed and deployment-speed performance impact.

[Panman82]

Ok, so then these "extra" files are only loaded for build tree purposes and not included in the build output. Correct?

[ef4]

No, they're included in the build output(the /dist/assets folder) but unused ones won't be downloaded to user's browsers.

[Panman82]

Must admit, I'm not too excited to learn about this. There is a lot of cruft in the upstream package that could be included. Additionally, this could be used as an attack vector, an (unexpected) exploitable file simply on the server is enough to cause problems. Is it possible to make dynamic imports that use build-time data a bit more direct? Basically process the import at build-time to get the specific files needed? I'm thinking about @embroider/macros specifically. Additionally, does this include importSync()?

[ef4]

Is it possible to make dynamic imports that use build-time data a bit more direct? Basically process the import at build-time to get the specific files needed?

That is most of what the build is already doing. It's just that when people use runtime variables it is literally uncomputable to know which files are needed are which are not, so it becomes like a glob pattern against the filesystem. We do enforce that the static part of the pattern limits things to a specific directory in a specific package, see test coverage for the illegal cases here:

https://github.com/ef4/ember-auto-import/blob/6fc0e75aa0e663ca40296100507258133571be0c/packages/ember-auto-import/ts/tests/splitter-test.ts#L222-L246

It's convenient to say:

function getFlavor(whichOne) {
  return import(`ice-cream/flavors/${whichOne}`)
}

and as long as the ./flavors directory contains only flavors you'd be OK with people using, then no additional sanitation is needed. If you care though, then yes, like any use of user-controlled input it needs to be validated.

When you really want explicit control over all the choices, you can always be explicit instead:

function getFlavor(whichOne) {
  switch (whichOne) {
    case 'vanilla': 
      return import('ice-cream/flavors/vanilla');
    case 'chocolate':
      return import('ice-cream/flavors/chocolate');
    default:
      throw new Error(`unknown flavor ${whichOne}`);
  }
}

Since that example contains no "wildcard" patterns, it will only bundle the explicit choices that are mentioned. Any others won't even appear in /dist.

[Panman82]

Sure, and I noticed that you can help limit what is included by specifying more of the path statically (ice-cream/${whichOne} vs ice-cream/flavors/${whichOne}). It's just that one of the benefits I was able to provide prior to ember-auto-import was giving the consumer control over which optional assets would be included in the build output. Now it won't matter, all of the optional assets will be in the output no mater what configuration options are given. Some people are picky about build size... /rant

[ef4]

I guess maybe you're talking about https://github.com/froala/ember-froala-editor? There are still two ways you could give users control over what features they're getting.

One is to use @embroider/macros to control the loading of those features. It looks like you're already pretty close here:

https://github.com/froala/ember-froala-editor/blob/da197efd1441c512009952e59fae1d0f38514855/addon/components/froala-editor.js#L14-L34

You can use the each macro to unroll those loops at build time:

import { each, importSync, getOwnConfig } from '@embroider/macros';
 for (const plugin of each(getOwnConfig().pluginsToImport)) {
  importSync(plugin);
}

Assuming your config.pluginsToImport is a list like ['froala-editor/js/plugins/a', 'froala-editor/css/plugins/b'], the each macro is supposed to replace the above with:

importSync('froala-editor/js/plugins/a');
importSync('froala-editor/css/plugins/b');

I think that will result in a fully-static build that doesn't even emit the unused plugins.

The second option, which would reduce your API surface area and let users lean more heavily on the upstream Froala editor documentation, would be to move froala-editor from dependencies to peerDependencies and tell users to manage the plugin importing directly.

[Panman82]

Yep! Funny you should link to that particular code, I was going to ask why a "standard" dynamic import() doesn't work here?? Using importSync() seems to work though, so I went with it. But I don't think it has to turn into require() statements...

Had no idea the each() was offered (didn't look at the macro src), but what you describe is exactly what I was hoping for! 🎉 I'll surely look into using that instead. Is a for of loop preferred, or would it better to be each().forEach()?

If I remember right, importSync(plugin) didn't work because it wanted a "string literal". I assume it has the same requirements as embroider dynamic import() with static package names at minimum, but didn't dig too far.

Yeah, the other thing I considered was actually having the consumer extend the component to import() the plugins and such that the needed (most will extend the component for other reasons anyway). But I think it's kind of nice that the addon does all of this for them... (since it has been doing so up to this point) 🤷‍♂️

[ef4]

I was going to ask why a "standard" dynamic import() doesn't work here??

It does, but you need some place to absorb the asynchronicity, and we don't have top-level await yet.

In fact I didn't notice until you mentioned it, but the async wrapper you have around that code isn't doing anything, since everything is sync. You could keep the async wrapper and use standard import(), but then there is a potential race condition because nothing is waiting for that promise to resolve.

Is a for of loop preferred

Yes, it's mandatory, the each macro only works with that syntax, since it's really a syntactic transformation, not a true runtime function.

If I remember right, importSync(plugin) didn't work because it wanted a "string literal".

This is correct, it only supports string literal and template string literal (with a sufficiently static prefix), but in this case the each macro takes precedence and rewrites importSync(plugin) to the specific literals before importSync sees them.

[Panman82]

It does, but you need some place to absorb the asynchronicity, and we don't have top-level await yet.

Right, before changing to importSync() I had used await import() as shown in the MDN documentation but it didn't seem to be working. Also considered returning a Promise.all() so that things would load async too. Just didn't know the proper usage for import() in this case, whereas the examples for importSync() worked.

This is correct, it only supports string literal and template string literal (with a sufficiently static prefix), but in this case the each macro takes precedence and rewrites importSync(plugin) to the specific literals before importSync sees them.

Nice! I'm going to try this right meow. Thanks!

[Panman82]

No dice, it still wanted "string literals" so I move the package name. Ex:

import { each, importSync, getOwnConfig } from '@embroider/macros';
for (const asset of each(getOwnConfig().importAssets)) {
  importSync(`froala-editor/${asset}`);
}

However, it still seems to want to import everything from the package because I'm seeing build errors trying to bring in example html files. Ex:

ERROR in ./node_modules/froala-editor/index.html 1:0
Module parse failed: Unexpected token (1:0)
You may need an appropriate loader to handle this file type, ...
@ ../path/to/node_modules/froala-editor/ sync ^\.\/.*$ ./index.html

UPDATE: Worked around the build errors by importing from deeper in the package to avoid the example files. I'm able to confirm some of the assets not even mentioned are being included in the "chunk" files. And this is what the above code example compiled down to:

  for (const asset of (0, _runtime.each)((0, _runtime.config)("/path/to/ember-froala-editor").importJsAssets)) {
    emberAutoImportSync("froala-editor/js/${e}", asset);
  }

  for (const asset of (0, _runtime.each)((0, _runtime.config)("/path/to/ember-froala-editor").importCssAssets)) {
    emberAutoImportSync("froala-editor/css/${e}", asset);
  }

UPDATE UPDATE: Hmmm.... seems to think it is for run-time.?. Unsure how to work around this check, but I suspect it has something to do with not allowing straight variables and requiring string templates. https://github.com/embroider-build/embroider/blob/v0.48.1/packages/macros/src/babel/each.ts#L52-L64

UPDATE UPDATE UPDATE: It seems this is what I'm running into before the each() macro even has a chance to transpose those statements. https://github.com/ef4/ember-auto-import/blob/v2.2.4/packages/ember-auto-import/ts/analyzer-plugin.ts#L111

PS. Do you want to move this elsewhere? I feel like I highjacked this issue at this point, we're somewhat off-topic from the original (while still somewhat relevant). 🤷‍♂️

[sinankeskin]

Any update on this? Since both V8 and browsers support this I think it's time to add to ember-auto-import or embroider directly.

To be more specific, both locales and plugins don't work with embroider. I'm trying to convert one of my addon to v2.

https://day.js.org/docs/en/i18n/loading-into-nodejs https://day.js.org/docs/en/plugin/loading-into-nodejs

const importedLocale = await import(`dayjs/locale/${localeName}`);

const importedPlugin = await import(`dayjs/plugin/${plugin}`);

Both of these throws an error.

Thanks.

[ef4]

I'm not sure what you're asking about. This issue was about selectively limiting the scope of dynamic imports at build time.

We have support for dynamic import and have had it for a long time.

[sinankeskin]

Updated the first comment and here are the errors. I don't know if it helps.

First one is with import and second one is with import { importSync } from '@embroider/macros';

So, I thought this doesn't work with template string. I'm exactly asking limiting import but not build time instead on demand.

[sinankeskin]

To solve the selective import what's the equivalent included hook for v2 addon imports? @ef4

Sorry to hijack folks.