-
Notifications
You must be signed in to change notification settings - Fork 15
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
json-6 require not working? #48
Comments
Okay. I updated my one-shot test batch file to do this..
I added tests but it works okay from the command line.
I had to rename test/1.0.9-require.mjs to .fail to allow pushing... I'm not sure how to get it to work from npm... from just mocha it seems to work fine. https://github.com/d3x0r/JSON6/tree/master/test renaming the require test to .mjs instead of .mjs.fail
|
Hmm, if you're running it via mocha directly, I presume you've installed mocha globally? That could make a huge difference (and is unlikely to be an uncommon real-world scenario). I get the same Preliminary results with |
found deprecation
that was a long time ago... could wish they included 'and has been replaced with....' |
I'd maybe recommend copying the snippet of code from require.js that loads and parses the file and returns an object.... as a workaround. |
I took a quick gander at @babel/register hoping there was something quick I could glean and incorporate from that, but it's much more involved than I was hoping |
This is the module the error is thrown from... specifically the list of extensions internal - with no way to add another list This has the resolve logic which I think prefilters '.json' ... it looks like it supports file:, data:, and node: URL resolutions for filenames... (not really any help) |
I think the way to handle this now is: https://nodejs.org/api/esm.html#esm_hooks Looking at This works the same for me as calling mocha directly (via $> npm run test --experimental-specifer-resolution=node (it also works when appended to the npm script) |
that doesn't work for me... I didn't see any sign that using the other map would have helped any... that still doesn't relate to 'require.extensions' ? (you mispelled specifier above :) and for a while node was saying it couldn't take that option)
|
I think it's not working for you because I didn't misspell if (experimentalSpeciferResolution === 'node') { |
By the way, for the esm require, it looks like a custom transpiler loader is the way to go: https://github.com/nodejs/node/blob/master/doc/api/esm.md#transpiler-loader That doc explicitly says |
okay so do I need to do a @rollup/json6-plugin etc ? babel? |
that doesn't solve everything though - I do still read .jsox(json6) files for configuration files because of human editability... so it should really always be a dynamic sort of thing that wouldn't be included... |
So I'm about to update the test to be 'success is to throw an error', and update the readme about limitations. |
I think neither of those. Basically an esm version of the existing // package.json
{
"scripts": {
"test": "NODE_ENV=test mocha ./src --experimental-loader json-6/lib/es-loader"
}
} |
This should basically work... updating the projects own build to include that and require causes it to fail to import 'node_modules/mocha/bin/mocha' because it doesn't have an extension.... but again was able to run the tests as a one-off with the extra flags. and this fails for me SET NODE_OPTIONS=--experimental-loader=./lib/import.mjs && mocha --require chai/register-expect --require ./lib/require.js However, the new loader should generally work... it worked as below... This usage worked. import {default as config} from './1.0.9-require.json6'
console.log( config ); This method also worked. return import( "./1.0.9-require.json6" ).then( config=>{
config.default //... |
Revised the loader; works more as intended. The first other version took advantage that JSON6 is basically just JS, so I could just emit 'export default '; this uses JSON6 parser properly to decode that; adds JSON6 to the globalThis object though. |
This looks great—thanks! I've got an unrelated ESM error currently that's blocking me trying this, but I'll get back as soon as I've cleared that. |
Hmm, I'm getting an error:
(esm-alias-loader.mjs is from ilearnio/module-alias#59) Strangely, if I change the sequence of loaders (json-6's after alias-loader), json-6's loader appears to stomp alias-loader. |
Not sure why I would 'stomp' the other... every operation checks for extension of '.json6' before doing anything custom, otherwise it calls the default functions passed.
don't know where esm-alias-loader comes from... it doesn't seem to be part of module-alias Edit: |
You might check your version... I did bump the version to 1.1.1 (from 1.0.5) so the 1.0/1 change might be keeping you on a prior version.... that might have stmped everything. |
I did check the json-6 version (and posted it with a few other potentially relevant ones) 🙂 |
The error says it's expecting a string but got an object. I compared your getFormat function with the example from esm's doc, and they both return an object like return defaultGetFormat(url,context ); it's missing a 3rd argument which is present in the example: // Let Node.js handle all other URLs.
return defaultGetFormat(url, context, defaultGetFormat); Your other functions include the 3rd RE esm-alias-loader getting stomped: the first thing that comes to mind is the json-6 loader doesn't have/export a resolve function (which would pass along whatever it doesn't understand, and is the only hook esm-alias-loader uses). Maaaaybe that's causing the stomping? EDIT: I just noticed in the esm loader doc it says a
|
Adding a resolve to json-6's import loader didn't address the potential stomping, and adding that 3rd code addedimport fs from "fs"; // same as before
import url, { URL, pathToFileURL } from 'url';
import path from "path"; // same as before
const baseURL = pathToFileURL(`${process.cwd()}/`).href;
export function resolve(specifier, context, defaultResolve) {
const { parentURL = baseURL } = context;
const ext = path.extname(specifier);
// Node.js normally errors on unknown file extensions, so return a URL for
// specifiers ending in the json-6 file extension.
if (ext === '.json6') return { url: new URL(specifier, parentURL).href };
// Let Node.js handle all other specifiers.
return defaultResolve(specifier, context, defaultResolve);
} |
I added some logging to ems-alias-loader, and its
I also tried adding It can't be that only 1 loader is allowed because when esm-alias-loader is after json-6's, they both run. |
Well I did ignore(remove) the resolve because it just ended up calling the default resolve - I want basically the default node file resolution... nothing custom to do with the path. (though I suppose that mod-alias uses that) I wondered about that default function parameter... it could be that it's only allowed to have 1, and you need one that merges both together... |
from node\lib\internal\modules\esm\loader.js
getting the format doesn't chain... the defaultGetFormat is just the internal version, not some other loader... |
node module: ESM loaders next steps This is an experimental state, so probably it can be fixed. |
@d3x0r so actually, you comment above about how json6 is effectively just javascript gave me an idea, which I just verified—this is actually the only necessary piece for importing json6 files: // jsonlike-loader.mjs
import path from 'path';
const moduleLikeExts = [
'.json',
'.json5',
'.json6',
];
export function getFormat(url, context, defaultGetFormat) {
const ext = path.extname(url);
if (moduleLikeExts.includes(ext)) return { format: 'module' };
return defaultGetFormat(url, context, defaultGetFormat);
};
This tells node to handle the files as if they were normal modules, and voila! No getSource, transformSource, etc. You only need to tell Node the file is a module. |
Okay1) I'm not sure why that would really behave any different; other than not intercepting the getSource or Transform calls and doing things with them.... they would only be used for .json6 files (I could add .json5 too). But, that doesn't help https://github.com/d3x0r/jsox which deviates from JS and requires its own transform... |
https://github.com/nodejs/node/blob/master/lib/internal/process/esm_loader.js#L42 node only uses one of the command line options (first? last?) It's right now even if you specify more than one it only uses one... this routine should at least be updated to use all --experimental-loader options.... would seem to me, that even if this was the simplest method, it still wouldn't work with other loaders.... |
Sorry, I'm not sure exactly what the problem is, but it appears json-6/lib/require doesn't work (and I'm seeing that require.extensions is deprecated).
When I try to import a
.json6
file and run it with mochajs/mocha (which I think uses standard-things/esm), I get the following error:I've included
json-6/lib/require
in mocha's--require
and confirmed the file is executed; however, the function set for'.json6'
never executes.I see that there is specific mention of JSON6 in esm's readme, so I tried adding the cited config to my package.json, but to no effect:
The text was updated successfully, but these errors were encountered: