-
-
Notifications
You must be signed in to change notification settings - Fork 106
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
Fix ERR_UNSUPPORTED_DIR_IMPORT for gulpfile.mjs #217
Conversation
Import directory testcase mentioned above:(a) My testcase was simple, I create an // repo structure
/
.d/
src/
gulpfile.mjs/
index.mjs
a/
files.txt
b/
x.conf
c.txt // gulpfile.mjs/index.mjs file
import gulp from 'gulp'
import path from 'path'
const dir = [
path.join('..', 'a'),
path.join('..', 'b/*.conf'),
path.join('..', 'c.txt')
]
const dest = '../.d/'
export function copy() {
return gulp.src(dir, { base: '..' })
.pipe(gulp.dest(dest))
}
export default gulp.series(copy) From |
One small problem appeared. If users set
On the bright side, if this PR gets refused. We can still organize |
@ngdangtu-vn Does this change the behavior people expect from the import syntax in node? I don't really want to define our own behavior for that. |
Yes and no:
Therefore, this patch is still safely to merge. |
@ngdangtu-vn I believe this needs conflicts resolved since I merged #239 |
@phated It has been too long so I don't really remember what I did back then :)) I can resolve this but do we really need this PR anymore? |
That's fine. I wasn't sure I wanted to include this so we can just close it out. |
What is this PR for?
This is a ERR_UNSUPPORTED_DIR_IMPORT fixed patch. (And did a bit code refactor in
require-or-import.js
file)Before this patch, gulp-cli won't be able to read directory URL for ESM mode. For example:
The only solution is append
index.mjs
to the directory. And I fixed it.Was it tested yet?
Almost everything.
gulpfile.mjs
directory -> Tested directly in my own repo (a)gulpfile.mjs
directory -> I have no idea how to do it, so this task is under researchingIf it possible, I would like to merge this patch to gulp-cli.