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

Support nested options #4

Closed
wants to merge 5 commits into from
Closed

Conversation

sttk
Copy link
Contributor

@sttk sttk commented Jun 30, 2019

This function will be used in vinyl-fs to solve the issue gulp#2288.

@phated
Copy link
Member

phated commented Jun 30, 2019

Unfortunately this won't solve the sourcemap issue. I will try to follow up on that when I have some free time.

@sttk
Copy link
Contributor Author

sttk commented Jun 30, 2019

@phated This pr only enables to specify advanced sourcemap options.
Moreover, we need to modify vinyl-fs and vinyl-sourcemap to write outputs by these options in source map files.

gulp.src('src/**/*.js', {sourcemaps: true})
  .pipe(gulp.dest('dist', {
    sourcemaps: { destPath: '.',  sourceRoot: '../src' }  // Enable to specify nested options like this.
  })); 

At https://github.com/gulpjs/vinyl-fs/blob/master/lib/dest/sourcemap.js#L11, these nested options can be gotten like following.

var srcMap = optResolver.resolve('sourcemaps', file);  // => { destPath: '.',  sourceRoot: '../src'  }

@3cp
Copy link

3cp commented Jun 30, 2019

Just a thought, no matter how is it fixed underneath, in this surface API:

gulp.dest('dist', {
    sourcemaps: { destPath: '.',  sourceRoot: '../src' }
  });

The sourceRoot is unnecessary, because you can automatically figure out sourceRoot based on the first arg to gulp.dest which is 'dist', plus the original file.base in vinyl-fs file itself.

I would suggest gulp to get the sourceRoot right by default for both gulp.dest('dist', {sourcemaps: true}) and gulp.dest('dist', {sourcemaps: '.'}).

@phated
Copy link
Member

phated commented Jul 1, 2019

I think if we are supporting nested options, we are complicating our options too much. Gulp was never intended to have the complete set of gulp-sourcemaps; it is only the initialization. End-users are expected to combine other plugins for advanced features.

@phated phated closed this Jul 1, 2019
@phated
Copy link
Member

phated commented Jul 1, 2019

@sttk I created a new tool for updating the repository scaffold. I'll need to show you that some time.

@sttk sttk deleted the support_nested_options branch February 12, 2022 08:39
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