ym-project / gulp-esbuild

gulp plugin for esbuild bundler
MIT License
42 stars 7 forks source link

Not taking input from gulp's pipes #5

Closed V-ed closed 3 years ago

V-ed commented 3 years ago

gulp-ts-alias rewrites the paths of imports to use TypeScript's paths configurations, and this plugins works as I can see because it does the right output when I comment out your plugin.

Does your plugin uses what's in the pipeline of gulp to send it to esbuild or does it uses the source files? That might be my issue (or I might just not know gulp enough yet...)

Here's how I configured my gulp tasks (stripped down) :

Example working path rewrites (but with .ts output files) ```js const ts = require('gulp-typescript'); const alias = require('gulp-ts-alias').default; const gulpEsbuild = require('gulp-esbuild'); const tsProject = ts.createProject('./tsconfig.json'); function esbuild() { return ( tsProject .src() // @ts-ignore .pipe(alias({ configuration: tsProject.config })) // .pipe( // gulpEsbuild({ // platform: 'node', // // bundle: true, // }), // ) .pipe(gulp.dest(tsProject.config.compilerOptions.outDir)) ); } ```
Example where your plugins doesn't seem to use gulp-ts-alias's output (js outputted, but bad paths in output files) ```js const ts = require('gulp-typescript'); const alias = require('gulp-ts-alias').default; const gulpEsbuild = require('gulp-esbuild'); const tsProject = ts.createProject('./tsconfig.json'); function esbuild() { return ( tsProject .src() // @ts-ignore .pipe(alias({ configuration: tsProject.config })) .pipe( gulpEsbuild({ platform: 'node', // bundle: true, }), ) .pipe(gulp.dest(tsProject.config.compilerOptions.outDir)) ); } ```

I need to use that other gulp plugin since currently, esbuild does not rewrite paths unless we use the bundle property, which I do not want to use for my backend application.

Any help will be appreciated! Thanks in adance!

ym-project commented 3 years ago

Hi @V-ed! This plugin read entry files from file system, not from pipeline. I tried to fix it via esbuild stdin api earlier. There were some problems but it worked. I'll think how to implement it.

V-ed commented 3 years ago

Very interesting, glad that you got a working prototype, building backend applications in TypeScript with custom paths had been a nightmare for me in the past few days, with this addition my whole setup will be complete (from dev to prod)!

Thanks for the work you're putting into this, esbuild is awesome and being able to leverage its power into gulp is a godsend!

ym-project commented 3 years ago

I have implemented. You can install test version by command npm i gulp-esbuild@piping-feature. Also you should use export const {pipedGulpEsbuild} = require('gulp-esbuild') to enable piping. You can see a simple example.

It should be tested before I merge this feature to master. I checked basic opportunities it's ok.

ym-project commented 3 years ago

Why did I do separate export for this?

Default export uses entryPoints option. I just add files to array and esbuild bundles it himself. In piped version plugin uses stdin build api. For this api I build every file as separate file. So build time will be increased a little.

Therefore all these features can not be combined in one export. Or I just don't understand how to do this yet.

V-ed commented 3 years ago

I'll try it in the next few hours, in the meantime, I have looked at your piped example (nice to see that you took the tsconfig.json from my most recent project, my testing will be literally how I'll use it haha) and I had a small question

In your piped example, you have a typescript file and use other plugins (such as gulp-ts-alias), but as this comment on esbuild says and based on what I can read in the stdin api doc, shouldn't we need to explicitly set the loader in this case? stdin has no file extension data, so I guess it would make sense, for typescript, to add the loader property to be ts, which you didn't do.

If it works like that, awesome! I'll test it and come back with results on how it went!

V-ed commented 3 years ago

Why did I do separate export for this?

Default export uses entryPoints option. I just add files to array and esbuild bundles it himself. In piped version plugin uses stdin build api. For this api I build every file as separate file. So build time will be increased a little.

Therefore all these features can not be combined in one export. Or I just don't understand how to do this yet.

I am fine with a different export, as long as it is explicitly said somewhere in your readme that by default it does what (currently loads files from filesystem, not gulp's pipes). When I think of a gulp plugin, I imagine that by default, it takes the content from the pipes, and that is what lead to my initial confusion.

About the question of building every file as a separate file, I think someone else already had this question, and there seems to be an option to bundle with stdin using a sourceFile option, I know I didn't want to bundle my app but I'll test few cases to see if that sourceFile options might work for my case without bundling.

Thanks for looking into this so quickly!

V-ed commented 3 years ago

I tested your pipedGulpEsbuild function and it does work for piping files to esbuild for transpiling purposes, and it still is faster than regular tsc in my case, but there is a significant problem remaining : The folder structure is not kept, meaning that relative imports fails in subfolders

source output files good output content, wrong folder structure
image image image

About the question of building every file as a separate file, I think someone else already had this question, and there seems to be an option to bundle with stdin using a sourceFile option, I know I didn't want to bundle my app but I'll test few cases to see if that sourceFile options might work for my case without bundling.

I can't test the stdin properties in the current state as you omit its value in your code

image

ym-project commented 3 years ago

In your piped example, you have a typescript file and use other plugins (such as gulp-ts-alias), but as this comment on esbuild says and based on what I can read in the stdin api doc, shouldn't we need to explicitly set the loader in this case? stdin has no file extension data, so I guess it would make sense, for typescript, to add the loader property to be ts, which you didn't do.

Without loader in stdin option you will get a syntax error if there are non-js syntaxis. And global loader option doesn't help to suppress this error.

When I think of a gulp plugin, I imagine that by default, it takes the content from the pipes, and that is what lead to my initial confusion.

You're right. But if you see on another plugin like webpack-stream, you will see what this plugin reads files from file system too (as I understand). I guess for bundlers it's ok.

I can't test the stdin properties in the current state as you omit its value in your code

Yes. Just user shouldn't have an access to several properties within the framework of the plugin.

ym-project commented 3 years ago

The folder structure is not kept, meaning that relative imports fails in subfolders

I updated gulp-esbuild@piping-feature version. Now it should keep file structure.

V-ed commented 3 years ago

Without loader in stdin option you will get a syntax error if there are non-js syntaxis. And global loader option doesn't help to suppress this error.

Uh, interesting, I didn't specify a loader yet it still worked to transpile to js from ts, ah well, I'll keep that in mind

You're right. But if you see on another plugin like webpack-stream, you will see what this plugin reads files from file system too (as I understand). I guess for bundlers it's ok.

Not gonna lie, I'm not at all a master of gulp nor do I have a lot of experience, so while I imagined all gulp related tasks to use piping, it does make sense that some plugins go for the route of file system only (such as bundlers, has you said). I'm happy to see a working version from pipes for esbuild tho, as this means I can safely use other plugins with it!

Yes. Just user shouldn't have an access to several properties within the framework of the plugin.

Hehe yes, plugin safety after all, it just meant that I couldn't easily test if some stdin options could work in my setup ;)

The folder structure is not kept, meaning that relative imports fails in subfolders

I updated gulp-esbuild@piping-feature version. Now it should keep file structure.

Awesome, I'll try it right away!

V-ed commented 3 years ago

The folder structure is not kept, meaning that relative imports fails in subfolders

I updated gulp-esbuild@piping-feature version. Now it should keep file structure.

Seems to be working perfectly in my use case! Thanks a lot!

ym-project commented 3 years ago

I will try to check most cases today or tomorrow. If it's ok I will merge to master (version 0.4.0). And then I will change plugin API (version 0.5.0) to simplify usage. It will look like

const {createGulpEsbuild} = require('gulp-esbuild')
const gulpEsbuild = createGulpEsbuild({
    watch: boolean, // for watch mode
    pipe: boolean, // to enable piping
    ... // for future features
})
V-ed commented 3 years ago

I will try to check most cases today or tomorrow. If it's ok I will merge to master (version 0.4.0). And then I will change plugin API (version 0.5.0) to simplify usage. It will look like

const {createGulpEsbuild} = require('gulp-esbuild')
const gulpEsbuild = createGulpEsbuild({
    watch: boolean, // for watch mode
    pipe: boolean, // to enable piping
    ... // for future features
})

Seems good, although if you were to simplify your usage, I'd recommend to add the ability to set esbuild's options straight from the createGulpEsbuild instead of returning the function object that is able to set the options

In other words, that :

const {createGulpEsbuild} = require('gulp-esbuild')
const gulpEsbuild = createGulpEsbuild({
    watch: boolean, // for watch mode
    pipe: boolean, // to enable piping
    ... // for future features
})

function build() {
    return src('./src/index.js')
        .pipe(gulpEsbuild({
            outfile: 'outfile.js',
            bundle: true,
        }))
        .pipe(dest('./dist'))
}

could be, for example :

const {createGulpEsbuild} = require('gulp-esbuild')

function build() {
    return src('./src/index.js')
        .pipe(createGulpEsbuild({
            watch: boolean, // for watch mode
            pipe: boolean, // to enable piping
            buildOptions: {
                outfile: 'outfile.js',
                bundle: true,
            },
            ... // for future features
        })
        .pipe(dest('./dist'))
}

Just my two cents! You do as you wish :)

ym-project commented 3 years ago

Released https://github.com/ym-project/gulp-esbuild/releases/tag/v0.4.0.

V-ed commented 3 years ago

Awesome! I even saw that you added an extra notice in the readme, which clears further confusions :)

I'll consider this issue fixed then, thanks for the support! 🎉

aunruh commented 1 year ago

hey the example link is offline :/. and the linked-to example on the release page doesn't contain pipedGulpEsbuild

ym-project commented 1 year ago

hey the example link is offline :/

Hi @aunruh! About what link do you talk?

aunruh commented 1 year ago

https://github.com/ym-project/gulp-esbuild/tree/stdin-build-api/examples/piping

this one!

ym-project commented 1 year ago

https://github.com/ym-project/gulp-esbuild/tree/stdin-build-api/examples/piping

this one!

This issue is old and strin-build-api branch doesn't exist. Actual branch is v4 and example is here https://github.com/ym-project/gulp-esbuild/tree/v0/examples/piping