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

Add empty loader #180

Closed
wants to merge 1 commit into from

Conversation

viankakrisna
Copy link
Contributor

this is useful in case we want to have an empty string as the result of importing a module, for example: while running tests we don't want to emit files or load the full contents of the imported file to the memory.

@viankakrisna viankakrisna force-pushed the feature/emptyloader branch from 29358af to 129ecac Compare June 15, 2020 03:52
@floydspace
Copy link
Contributor

Hey @viankakrisna, maybe the result of the loader should be null or undefined? not all file types return a string

@viankakrisna
Copy link
Contributor Author

@floydspace i think that could be a separate loader. the use case for this is to replace the loaders that's usually output string to js like file, text, base64, and dataurl, but we don't want to use them in some cases. probably emptystring is a better name for this loader?

@floydspace
Copy link
Contributor

@viankakrisna as long as there is already text loader we could name it emptytext or mocktext

@evanw
Copy link
Owner

evanw commented Jun 20, 2020

I've been thinking about this for a bit. I agree that something like emptytext would be a better name for this since I'd expect something named empty to cause the file to be empty.

However, I think this is starting to get at more complicated loader scenarios, and I want to think more carefully about loaders like this to avoid adding something that doesn't fit well into the overall design.

I've been thinking about loader chaining recently. Really this loader is two concepts: forcing the input file to be empty and packaging up a file as a raw string. It would be nice if you could say --loader=empty,text instead to avoid needing to special-case the combination of these two concepts.

This has come up for other loaders in the past too. For example, at one point #107 included code for a SVG loader, except it combined the two concepts of loading an SVG file and packaging up the file as a data URL. The SVG loader would be a natural place to add SVG-specific optimizations over time (e.g. minification). But if you wanted the SVG file as a raw string instead of a data URL, you'd be out of luck. Or at least you'd have to add a lot of extra options to the SVG loader for different output formats, or you'd have to make lots of SVG loaders for different combinations of concepts. It seems like a cleaner design to me to be able to do something like --loader=svg,dataurl and keep the concepts separate by chaining loaders. So I'm thinking this is the direction I'd like to take loaders.

However, I want to think carefully about how this works in the general case. I also need loaders to support HTML (#96) and CSS (#20). I'm not sure how I feel about requiring all loaders to ultimately produce a JavaScript file. It might be nice to be able to just bundle CSS to CSS or just minify some HTML, for example. Some loaders also need to produce multiple output files (e.g. the file loader, also CSS modules in the future). So I haven't yet decided what the right interface between loaders in a loader chain is.

I also haven't yet done research into how other bundlers handle their loader APIs, and how they solve these problems. Ideally that research would be done as part of this too.

@evanw evanw force-pushed the master branch 2 times, most recently from 5b6b355 to 6e3ef52 Compare July 12, 2020 07:45
@evanw evanw force-pushed the master branch 2 times, most recently from 9415613 to 98858c0 Compare July 25, 2020 22:03
@evanw
Copy link
Owner

evanw commented Dec 31, 2020

This specific scenario and other similar scenarios are already addressable with the on-load plugin API which was added since this PR was opened. I'm closing this because I don't think this empty file feature should be built-in given that it's pretty specific and is already covered by plugins.

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

Successfully merging this pull request may close these issues.

3 participants