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

Reconsider FluentResource instantiation #400

Closed
stasm opened this issue Jul 22, 2019 · 4 comments
Closed

Reconsider FluentResource instantiation #400

stasm opened this issue Jul 22, 2019 · 4 comments

Comments

@stasm
Copy link
Contributor

stasm commented Jul 22, 2019

In fluent 0.12, FluentResource is a subclass on Map. It exposes a static fromString method which can be used to parse a new resource from a string. In #380, which will be released the upcoming fluent 0.13, I changed FluentResource to extend Array and I kept fromString.

let resource = FluentResource.fromString("foo = Foo");

I don't fully remember why there is fromString in the first place. I think it's related to how transpilers can't handle extending native constructors back to ES5. But since we don't target ES5 any more (#133), perhaps we don't need a separate static method?

The reason why I wonder is that in a few examples of @zbraniecki's WebIDL experiemnts, I noticed he used a different API:

let resource = new FluentResource("foo = Foo");

Should we consider switching to it? And related: should we keep FluentResource as a subclass of Array, or perhaps make it an independent class with a semi-private field called _messages?

CC @Pike.

@Pike
Copy link
Contributor

Pike commented Jul 22, 2019

I'm fine with a constructor.

If we'd make FluentResource a standalone class, I'd stick to what we do in the reference, and call the list body.

That'd match what I expect to end up doing in python, btw. I'm thinking along the lines of

def Resource(content):
   return parse(content)

to create a Resource class, w/out having to actually implement a runtime resource class ;-) . That will have a body property, being a list of entries.

@stasm
Copy link
Contributor Author

stasm commented Jul 22, 2019

Good idea with body.

I think I'd prefer to not subclass Array at this point. I'm not seeing any perf impact, and I like it that we'd be no longer tempting people to slice(), push or even query length.

Let's wait for @zbraniecki to chime in, too.

@zbraniecki
Copy link
Collaborator

yeah, sounds good to me. I'd consider the guts of FluentResource to not be public, much like FluentPattern.

@stasm
Copy link
Contributor Author

stasm commented Jul 23, 2019

Fixed in #401 on the release branch.

@stasm stasm closed this as completed Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants