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

Not taking input from gulp's pipes #5

Closed
V-ed opened this issue Feb 4, 2021 · 19 comments
Closed

Not taking input from gulp's pipes #5

V-ed opened this issue Feb 4, 2021 · 19 comments
Labels
enhancement New feature or request

Comments

@V-ed
Copy link

V-ed commented Feb 4, 2021

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)
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)
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!

V-ed added a commit to V-ed/node-api-template that referenced this issue Feb 4, 2021
The only remaining issue is with the build step, which gulp-esbuild doesn't use the gien aliases output from the pipes.
Ref : ym-project/gulp-esbuild#5
@ym-project
Copy link
Owner

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

V-ed commented Feb 5, 2021

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!

@V-ed V-ed changed the title Can't get it to work with gulp-ts-alias (nor any of typescript's paths plugins handlers) Can't get it to work with gulp-ts-alias Feb 5, 2021
@V-ed V-ed changed the title Can't get it to work with gulp-ts-alias Not taking input from gulp's pipes Feb 5, 2021
@ym-project
Copy link
Owner

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

ym-project commented Feb 5, 2021

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.

@ym-project ym-project added the enhancement New feature or request label Feb 5, 2021
@V-ed
Copy link
Author

V-ed commented Feb 5, 2021

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

V-ed commented Feb 5, 2021

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

V-ed commented Feb 5, 2021

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

ym-project commented Feb 6, 2021

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

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

V-ed commented Feb 6, 2021

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

V-ed commented Feb 6, 2021

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

ym-project commented Feb 6, 2021

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

V-ed commented Feb 6, 2021

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

@V-ed
Copy link
Author

V-ed commented Feb 7, 2021

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! 🎉

@V-ed V-ed closed this as completed Feb 7, 2021
@aunruh
Copy link

aunruh commented Sep 13, 2023

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

@ym-project
Copy link
Owner

hey the example link is offline :/

Hi @aunruh! About what link do you talk?

@aunruh
Copy link

aunruh commented Sep 16, 2023

@ym-project
Copy link
Owner

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

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

No branches or pull requests

3 participants