-
Notifications
You must be signed in to change notification settings - Fork 239
Add support for import attributes and JSON modules #1300
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
Conversation
| JSModuleLoaderFunc *module_loader, void *opaque); | ||
|
|
||
| /* same as JS_SetModuleLoaderFunc but with import attributes support */ | ||
| JS_EXTERN void JS_SetModuleLoaderFunc2(JSRuntime *rt, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: do we want to keep the 2 suffix function a la bellard or make it the one and only loader function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No super strong opinion. A polygamist would say "two is better than one", a programmer the inverse. What would a polygamous programmer say though?
If you want me to make the call: only one.
66f4787 to
3357c2a
Compare
| cstr = JS_AtomToCStringLen(ctx, &cstr_len, tab[i].atom); | ||
| if (!cstr) { | ||
| ret = -1; | ||
| break; | ||
| } | ||
| if (!(cstr_len == 4 && !memcmp(cstr, "type", cstr_len))) { | ||
| JS_ThrowTypeError(ctx, "import attribute '%s' is not supported", cstr); | ||
| ret = -1; | ||
| } | ||
| JS_FreeCString(ctx, cstr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Future enhancement (because there are lots of places where we have this kind of atom-to-string munging):
bool JS_AtomStrEq(JSRuntime *rt, JSAtom atom, const char *s, size_t n);Name TBD and I suppose for parity you'd also want one to compare atomized integers.
| JSModuleLoaderFunc *module_loader, void *opaque); | ||
|
|
||
| /* same as JS_SetModuleLoaderFunc but with import attributes support */ | ||
| JS_EXTERN void JS_SetModuleLoaderFunc2(JSRuntime *rt, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No super strong opinion. A polygamist would say "two is better than one", a programmer the inverse. What would a polygamous programmer say though?
If you want me to make the call: only one.
3357c2a to
944720e
Compare
|
I had a change of heart and chose to keep both the old and the new APIs. We can revisit later. |
Ref: bellard/quickjs@f10ef29