Skip to content
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

Some methods missing #24

Closed
Ehesp opened this issue Jan 26, 2023 · 9 comments
Closed

Some methods missing #24

Ehesp opened this issue Jan 26, 2023 · 9 comments

Comments

@Ehesp
Copy link
Contributor

Ehesp commented Jan 26, 2023

It seems as though some classes, for example Headers is missing some methods:

image

class Headers {
  external factory Headers([dynamic init]);
}

extension PropsHeaders on Headers {
  void append(String name, String value) =>
      js_util.callMethod(this, 'append', [name, value]);

  void delete(String name) => js_util.callMethod(this, 'delete', [name]);

  @JS('get')
  @staticInterop
  String? mGet(String name) => js_util.callMethod(this, 'get', [name]);

  bool has(String name) => js_util.callMethod(this, 'has', [name]);

  @JS('set')
  @staticInterop
  void mSet(String name, String value) =>
      js_util.callMethod(this, 'set', [name, value]);
}
@Ehesp
Copy link
Contributor Author

Ehesp commented Jan 26, 2023

Hmm looks like it might be iterable methods not being supported/handled. If we can handle them in the build code (not sure where it is), I think the API could probably look like this:

Iterator keys() => js_util.callMethod(this, 'keys', []);
@js.JS()
@js.staticInterop
class Iterator {
  external factory Iterator();
}

extension PropsIterator on Iterator {
  IteratorNext next() => js_util.callMethod(this, 'next', []);
}

@js.anonymous
@js.JS()
class IteratorNext {
  external factory IteratorNext();
}

extension PropsIteratorNext on IteratorNext {
  bool get done => js_util.getProperty(this, 'done');
  dynamic get value => js_util.getProperty(this, 'value');
}

What do you think?

@Ehesp
Copy link
Contributor Author

Ehesp commented Jan 26, 2023

Hmm, you already have some iterator logic which extends JsArray. Not sure :D

@jodinathan
Copy link
Owner

There are different specifications even thought it is all JS.
JsArray, iterator etc are from Emacscript IIRC

I've created an issue regarding this

Maybe we should hardcode the missing methods?

@Ehesp
Copy link
Contributor Author

Ehesp commented Jan 26, 2023

I think hardcoding would be an OK approach, maybe just some custom extensions which are exported. Given the amount of work involved and how little the apis change I think it's probably the route to fastest solution?

If you could do one in a branch to demonstrate how to best do it, I'm happy to add them in places too.

@jodinathan
Copy link
Owner

In the case of Headers we would need to check the IDL JSON files where it is defined if it extend something like JsArray or JsMap then check in the builder why it is not recognizing and implementing.
For the sake of teaching the quirks I already found it in the fetch.json line 88. It has no inheritance, however, there is a iterable member that I was not sure if the builder takes that into any consideration.
Digging into the code I've found that if there is a member called iterable, the object should extend JsArray.
You can check what is going on there.

About the remaining methods and properties of JsArray, JsMap or any other JS object, just take JsArray as example and add the stuff into the extension.

@Ehesp
Copy link
Contributor Author

Ehesp commented Jan 31, 2023

Thanks - I'm not entirely sure how this works, but it looks as though it could be the builder which isn't adding the methods to the IDL files?

I see what you mean here, however there's no name and it's also missing the various methods.

{
            "type": "iterable",
            "idlType": [
              {
                "type": null,
                "extAttrs": [],
                "generic": "",
                "nullable": false,
                "union": false,
                "idlType": "ByteString"
              },
              {
                "type": null,
                "extAttrs": [],
                "generic": "",
                "nullable": false,
                "union": false,
                "idlType": "ByteString"
              }
            ],
            "arguments": [],
            "extAttrs": [],
            "readonly": false,
            "async": false
          }

@Ehesp
Copy link
Contributor Author

Ehesp commented Jan 31, 2023

FYI @jodinathan no clue if you're interested, but I'd be willing to support development of this package. My company is using it for a project - feel free to reach out on Twitter/email if interested!

@jodinathan
Copy link
Owner

FYI @jodinathan no clue if you're interested, but I'd be willing to support development of this package. My company is using it for a project - feel free to reach out on Twitter/email if interested!

Hi @Ehesp I've emailed you

@jodinathan
Copy link
Owner

@Ehesp I am closing this issue as it should be fixed in 0.1.0.

If you find more stuff missing please create a new issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants