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

Configuration option loader is ignored when type: "transform" #27

Open
karrikuivanen opened this issue Jun 9, 2021 · 10 comments
Open

Comments

@karrikuivanen
Copy link

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!

@folke
Copy link
Owner

folke commented Jun 9, 2021

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!

@karrikuivanen
Copy link
Author

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.

@marcelbeumer
Copy link

@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"?

export const loaders: Record<string, Loader> = {

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.

@folke
Copy link
Owner

folke commented Jun 14, 2021

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
Copy link
Owner

folke commented Jun 14, 2021

@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.

@marcelbeumer
Copy link

marcelbeumer commented Jun 14, 2021

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?

Module._extensions[ext] = (mod: PatchedModule, filename: string) => {

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.

@folke
Copy link
Owner

folke commented Jun 14, 2021

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

@marcelbeumer
Copy link

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

@marcelbeumer
Copy link

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

Messages crossed. Thanks!

@zemlanin
Copy link

zemlanin commented Feb 6, 2022

Hi. Is fix still coming?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants