Replace imports of modules/cal with modules/std/cal
fixes #1119
From the research I did I only found the wrong cal imports inside range.ts .
@scotttrinh , @jaclarke I'm really not sure should the driver behave differently depending on the server version,
I am not sure does the driver should return std::cal::local_date etc as the typename of the codec or it is okay if it returns cal::local_date in the SCALAR_CODECS map also for server versions >=6?
@jaclarke just told me we don't use the typename info from the codecs anywhere so I guess we don't need to make any changes there. Or we can update cal:: with std::call just to be consistent with the current behavior instead with the "legacy". But in general, it doesn't matter.
@scotttrinh I believe for the replace part inside edgeql-js.ts file you had sth like this in mind, but I don't think we really need this, because I believe in the future if we add anything we will use the correct path and also because std folder that is generated for example can have: ["cal", "enc", "fts", "math", "net"], but only cal, fts, math, and pg are actually moved, so I kinda think we do here unnecessary job...
// write syntax files
const syntaxOutDir = path.join(outputDir);
if (!(await exists(syntaxOutDir))) {
await fs.mkdir(syntaxOutDir);
}
const syntaxFiles = syntax[target];
if (!syntaxFiles) {
throw new Error(`Error: no syntax files found for target "${target}"`);
}
// libs that exists inside modules/std
const stdLibs: string[] = [];
if (version.major > 5) {
const stdPath = path.join(prettyOutputDir, "modules", "std");
const filenames = await fs.readdir(stdPath);
for (const fname of filenames) {
const fullPath = path.join(stdPath, fname);
const fileStat = await fs.stat(fullPath);
if (fileStat.isFile()) {
const libName = path.parse(fname).name;
stdLibs.push(libName);
}
}
}
for (const f of syntaxFiles) {
const outputPath = path.join(syntaxOutDir, f.path);
written.add(outputPath);
const oldContents = await readFileUtf8(outputPath)
.then((content) => content)
.catch(() => "");
let newContents = headerComment + f.content;
// in server versions >=6 cal, fts, math and pg are moved inside std module
if (version.major > 5) {
stdLibs.forEach((lib) => {
newContents = newContents.replace(
`modules/${lib}`,
`modules/std/${lib}`,
);
});
}
if (oldContents !== newContents) {
await fs.writeFile(outputPath, newContents);
}
}
I believe for the replace part inside edgeql-js.ts file you had sth like this in mind, but I don't think we really need this, because I believe in the future if we add anything we will use the correct path and also because std folder that is generated for example can have: ["cal", "enc", "fts", "math", "net"], but only cal, fts, math, and pg are actually moved, so I kinda think we do here unnecessary job...
It would be necessary if we added a new type, similar to multirange, that needed access to, for instance, math. We would need to import from modules/math for <= 5 and /modules/std/math for >= 6. I know it would interate over some no-op values like enc and net (which never lived in the global module namespace), but that's a tiny price to pay to make this code generic enough to not have to hardcode specific values into. Since this is a generation-time cost, and hopefully the cost of running String.prototype.replace is pretty minor, I'd still prefer to do it this way.
If we want to optimize this and encode some of our knowledge here, we can just iterate over the modules we know got moved, but then I'd want to have a comment here explaining to someone coming after us that these modules used to live in the global module namespace and where moved, so we must special case them, and still iterate over all four of those modules (cal, fts, math, and pg).
I'm really not sure should the driver behave differently depending on the server version, I am not sure does the driver should return std::cal::local_date etc as the typename of the codec or it is okay if it returns cal::local_date in the SCALAR_CODECS map also for server versions >=6?
I think we should return the unprefixed name for now since it works in both cases unless you've defined your own module with these exact module+type/function names. If we ever change the behavior here (not likely!) we can revisit. Or if users report issues where they're shadowing these modules on purpose and cannot get the TS driver to work, we can look into a workaround.