-
Notifications
You must be signed in to change notification settings - Fork 130
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 enable_import_extensions option #233
base: main
Are you sure you want to change the base?
Conversation
Thanks for this, Nicolas! I think that CI fails because the old TypeScript version being used doesn't understand If you were to bump TypeScript in packages/test-generated/package.json to a recent version, I think test-generated should pass, which will be a good proof that it works. Would you like to do so? CI failure log:
|
Hi @timostamm , Thanks for the feedback, sure! For now, i'm using a quick'n dirty script on my side to ensure all local imports has an extension. import * as fs from "fs";
import glob from "glob";
function add_import_extensions(er, files) {
files.forEach(file => {
let data = fs.readFileSync(file, {encoding: "utf8"});
data = data.replace(/from\ "\.\/(((?!\.js).)*)";$/gm, "from \"./$1.js\";");
data = data.replace(/from\ "\.\.\/(((?!\.js).)*)";$/gm, "from \"../$1.js\";");
fs.writeFileSync(file, data);
});
}
glob("./api/js/**/*.ts", {}, add_import_extensions);
glob("./api/js/**/*.js", {}, add_import_extensions); |
@nvandamme any updates on this? Since ts 4.7 released this is crucial |
Any update on this? ESM is becoming more widespread so would be nice to have this added. |
@timostamm are you happy with this implementation, or are there changes you want performed, I've just hit a case where this is now a blocker, thanks. |
for those who suffers and cannot wait until this gets merged, just run this after generation // proto-patch.js
import glob from "glob";
import fs from "fs";
const protoRoot = 'srv/proto';
glob(protoRoot + '/**/*.ts', async (err, files) => {
files.forEach(file => {
let content = fs.readFileSync(file, 'utf-8');
content = content.split('\n').map(s => s.replace(/^(import .+? from ["']\..+?)(["'];)$/, '$1.js$2')).join('\n');
fs.writeFileSync(file, content, 'utf-8')
});
}); |
any updates? |
@timostamm can we move it forward? without .js extension generated imports are broken for now |
@timostamm any update on when this can be merged? What's holding it back at the moment? |
@timostamm , if you still cannot get your test pipeline to work with this, please consider releasing this as experimental. You can make it obvious that there is no guarantee, if you name it |
Another version without npm dependencies while we await this feature to get merged. I use it as: #! /usr/bin/env node
import fs from 'node:fs';
import { dirname } from 'node:path';
import { fileURLToPath } from 'node:url';
const currentDir = dirname(fileURLToPath(import.meta.url));
const collectFiles = (directory, fileList = []) => {
fs.readdirSync(directory)
.map((innerFile) => `${directory}/${innerFile}`)
.forEach((file) =>
fs.statSync(file).isDirectory() ? collectFiles(file, fileList) : fileList.push(file)
);
return fileList;
};
const files = collectFiles(`${currentDir}/dist`).filter((file) => file.endsWith('.ts'));
files.forEach((file) => {
const content = fs.readFileSync(file, 'utf8');
const updated = content
.split('\n')
.map((s) => s.replace(/(} from ["']\..+(?<!\.js))(["'];)$/, '$1.js$2'))
.join('\n');
fs.writeFileSync(`${file}`, updated, 'utf-8');
}); |
Without the
I put in the error message for easy searching, but this PR is over two years old. Is there going to a release with it soon? |
I’m testing with the experimental node native typescript feature. In the past node required |
No description provided.