esbuild-runner icon indicating copy to clipboard operation
esbuild-runner copied to clipboard

Configuration option loader is ignored when type: "transform"

Open karrikuivanen opened this issue 3 years ago • 10 comments

Hi,

I noticed having type: 'bundle' in my esbuild-runner config broke my test coverage, so swapped it out to "transform".

This, however, caused my loader definition to be ignored.

Would it be possible to give type: 'transform' the same love you gave to bundle in #24 ?

Thanks!

karrikuivanen avatar Jun 09 '21 14:06 karrikuivanen

I thought that should work too, but I'll have a look. It's probably bailing out before the transform method is called with your custom loader config. I'll have a look!

folke avatar Jun 09 '21 17:06 folke

Sorry, I should have RTFM: Looks like loader works a bit differently when using the Transform API.

I believe it's this feature causing my issue, not your implementation.

/Users/karri/code/work/service/assets/img/image.png:1
�PNG

SyntaxError: Invalid or unexpected token

I think this issue can be closed now. I do still have the issue of losing code coverage reporting (nyc & mocha) when using esbuild, but that's out of scope for this issue.

karrikuivanen avatar Jun 10 '21 07:06 karrikuivanen

@folke running into the same thing wanting to transform code that does an import on a .css file (inside node_modules).

@karrikuivanen is right that for transform mode, there can only be one loader at a time (because there is one transform happening per file)

The list of "entry file" loaders is hardcoded here. What was the reason to comment out ".css"? https://github.com/folke/esbuild-runner/blob/1d38dd44eb6d72d5b838989a28b334eafeaef8cb/src/esbuild.ts#L41

Additionally, I understand that there is a check to prevent sources in node_modules to be transformed, but would it not make sense for JSON and CSS and so on to also be able to import from node_modules?

I don't have a strong opinion or suggestion, mainly sharing findings, but maybe be albe to fully configure the transform loaders including include/exclude?

For now I'll deal differently with this single .css file, transforming/importing just TS worked perfectly fine with esr.

marcelbeumer avatar Jun 14 '21 18:06 marcelbeumer

Have you tried configuring it with the loader option? It can be an object as with build. When transforming, we will only pass one loader, but we take the default loaders together with the configured loaders as input.

folke avatar Jun 14 '21 18:06 folke

@marcelbeumer the reason we dont transpile sources under node_modules is that it should not be needed.

tbh honest, I don't remember the exact reason why I didn't inlcude the css loader. Probably because I ran into an issue with some code. I'm happy to add it as a default again.

folke avatar Jun 14 '21 18:06 folke

Have you tried configuring it with the loader option? It can be an object as with build. When transforming, we will only pass one loader, but we take the default loaders together with the configured loaders as input.

Yes I tried with the loader option, but it didn't work, I think because here only the hardcoded loaders are wired? https://github.com/folke/esbuild-runner/blob/1d38dd44eb6d72d5b838989a28b334eafeaef8cb/src/hook.ts#L48

It works when I uncomment the .css in the hardcoded loaders, and disable the node_modules check for .css extension. I understand what you're saying about it should not be needed to transform (JS) in node_modules. However, I am importing a css file from a node_module (codemirror, but whatever) and expected it to pick it up, a bit webpack-esque, because when I use esbuild (without esr) to bundle the code, it allows importing css from node_modules and nicely creates a separate css bundle for me.

marcelbeumer avatar Jun 14 '21 19:06 marcelbeumer

Right, I know what's happening here. I'll work on a fix!

folke avatar Jun 14 '21 19:06 folke

Let me know if it helps to create a separate reproducible test case (esbuild vs esr bundle vs esr transform)

marcelbeumer avatar Jun 14 '21 19:06 marcelbeumer

Right, I know what's happening here. I'll work on a fix!

Messages crossed. Thanks!

marcelbeumer avatar Jun 14 '21 19:06 marcelbeumer

Hi. Is fix still coming?

zemlanin avatar Feb 06 '22 12:02 zemlanin