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

Why is the way 'this' passed was in different way in NamespaceExtension and ConstructorExtension? #6

Open
mmis1000 opened this issue Nov 21, 2020 · 5 comments

Comments

@mmis1000
Copy link

mmis1000 commented Nov 21, 2020

Looks like the ways arguments was passed will be altered depends on how you organize the extension?

Besides that.

The ConstructorExtension don't expose the object the method was on (a.k.a. ext.prototype) to the method.

While NamespaceExtension by default do that (ext itself was called as this).
Is this ever required?

Shouldn't the method be able to just refer the object that host it as ext (or whatever) if it must do it?

const ext = {
  method (self, arg1, arg2) { 
    return this.method2(self)
    // why do you want to use `this` to refer 'ext' here ?
    // Isn't it able to just do `this::ext:method2()`
    //   if it was passed in same way the class method do?
  },
  method2 (self, arg1, arg2) {}
}

someObj::ext:method()

And this breaks compatibility of namespace import and named import

// file a.js
export function A(self) {
  console.log(self.a)
}

// file b.js
import { ::A } from 'a.js'
import * as a from 'a.js'

const obj {
 a: 'hello'
}

obj::a:A() // this would work
obj::A() // this unfortunately not because it pass this as this instead of arguments[0]

That would makes refactor from namespace import to named import require a complete code rewrite because it breaks the semantic.

@mmis1000 mmis1000 changed the title Why is the way 'this' was passed in different way in NamespaceExtension and ConstructorExtension? Why is the way 'this' passed was in different way in NamespaceExtension and ConstructorExtension? Nov 21, 2020
@hax
Copy link
Member

hax commented Dec 3, 2020

They are designed for common use cases.

For constructors, the common use cases are reusing the prototype method.

For namespace objects, the common cases are using the first argument as this argument, which follow lodash/underscore, and many methods on built-in namespaces like Math.

About your example code, it actually works (with some syntax correction).

// file a.js
export function A(self) {
  console.log(self.a)
}

// file b.js
// import { ::A } from 'a.js'  // note this is the wrong syntax as current draft

import ::{A} from 'a.js' // this is the correct syntax as current draft

import * as a from 'a.js'

const obj {
 a: 'hello'
}

obj::a:A() // this would work
obj::A() // this actually works as current draft!

Note as current draft:

import {A} from 'a.js'
const ::A = A
obj::A() // wrong usage!

@mmis1000
Copy link
Author

mmis1000 commented Dec 3, 2020

Do import ::{A} from 'a.js' still expose the module itself as this?

If it isn't, then these two statement won't have the same semantic or be interchangeable?
(Although it is probably the same in current existing import statement)

Besides that, Is expose of namespace object intend to make pattern like this possible?

const encoder = new Encoder()
const result = input::encoder:encode(encodeOptions);

@hax
Copy link
Member

hax commented Dec 11, 2020

Do import ::{A} from 'a.js' still expose the module itself as this?

Yes.

// module m
export function plus1(x) { return x + this.one }
export const one = 1
import ::{plus1} from 'm'
1::plus1() // 2

But I don't think programmers would write code like that.

Of coz there is possibility someone really want to export a free method, in that case, they should use import {freeMethod} from 'mod'; const ::freeMethod = freeMethod. Currently there is no way to know whether a function is intended to be a free method, but maybe future proposal like explicit this param could help to solve that.

Besides that, Is expose of namespace object intend to make pattern like this possible?

const encoder = new Encoder()
const result = input::encoder:encode(encodeOptions);

Assume encoder is not a constructor, input::encoder:encode(encodeOptions) would behave as encoder.encode(input, encodeOptions), so yes, it should work as u expect.

@cshaa
Copy link

cshaa commented Jun 2, 2021

Do import ::{A} from 'a.js' still expose the module itself as this?

Yes.

// module m
export function plus1(x) { return x + this.one }
export const one = 1
import ::{plus1} from 'm'
1::plus1() // 2

This sounds like a terribly confusing “feature” to me. I would expect x::foo() to always roughly translate to foo.call(x). Having to think about when x is passed as this and when as an argument adds a lot of cognitive load which is, in my opinion, completely unnecessary and avoidable. If anyone, for whatever reason, prefered to write their code with x as the first argument, they could do so explicitly by using Extension.method.

In my opinion, this syntax:

import ::{ A } from 'a';

should just be a shorthand for this syntax:

import { A } from 'a';
const ::A = A;

@mmis1000
Copy link
Author

mmis1000 commented Jun 24, 2021

How about the reverse?
Anything "imported" from other file was treated as free method regards of how it was imported.

Which means.

// a
export function A () {
  console.log(this)
}

// b
import ::{ A } from 'a';
import * as a from 'a';
import { A as A1 } from 'a';
const ::A1 = A1

"a"::a:A()
"a"::A()
"a"::A1()

Either of them get the same result a.

It looks giantly unnatural to me that

If you want the extension to be imported from other file using shorthand, 
you now need to write receiver as first argument instead of `this`.

And this can also make tool chains happy due to:

If it only see the namespace import used as extension,
it can just happily shake it safely even without knowing contents of other file.
Because imports never use namespace object in extension

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

3 participants