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

Problems with with #55

Open
Haringat opened this issue May 23, 2022 · 12 comments
Open

Problems with with #55

Haringat opened this issue May 23, 2022 · 12 comments

Comments

@Haringat
Copy link

Haringat commented May 23, 2022

(aside from a slightly confusing issue title)

Imagine the following two files:
a.js:

import {Vector} from "mymath";

export function addWithoutOperatorOverloading(v1, v2) {
    return v1 + v2;
}
with operators from Vector;
export function addWithOperatorOverloading(v1, v2) {
    return v1 + v2;
}

b.js

import {addWithoutOperatorOverloading, addWithOperatorOverloading} from "./a.js";
import {Vector} from "mymath";

const v1 = new Vector(1, 2);
const v2 = new Vector(3, 4);
const r1 = addWithOperatorOverloading(v1, v2); // will work as intended
const r2 = addWithoutOperatorOverloading(v1, v2); // ???
const r3 = v1 + v2; // NaN because "with operators from" is not present in this file

what will r2 be? The with operators from statement in a.js is already parsed as addWithOperatorOverloading was used which is defined below that statement. Having the result depend on how far the file is parsed/executed would cause a lot of ambiguity and would be a hassle for the engine to sort out but having the statement have effects on higher lines in the file would clearly be unintentional.

And how are bundlers like rollup supposed to handle this? The following (naive, but in line with how bundlers work) approach would not be acceptable as it would cause intended leak of operator overloading into higher code:
bundle.js

import {Vector} from "mymath";

function addWithoutOperatorOverloading(v1, v2) {
    return v1 + v2;
}
with operators from Vector;
function addWithOperatorOverloading(v1, v2) {
    return v1 + v2;
}

const v1 = new Vector(1, 2);
const v2 = new Vector(3, 4);
const r1 = addWithOperatorOverloading(v1, v2); // will work as intended
const r2 = addWithoutOperatorOverloading(v1, v2); // ????
const r3 = v1 + v2; // suddenly the same as r1, clearly wrong

Here the value of r2 would be even more ambiguous as addWithoutOperatorOverloading is clearly called below the with operators from statement.

@matthew-dean
Copy link

Yeah, having the operator behavior change mid-block seems really bad. It should apply to the entire block scope (hoisted).

@Pandapip1
Copy link

How about throwing a syntax error if any with operators from x is not the first statement of a block? That way, at least with the second example, an error would be thrown.

I personally would prefer this proposal to be split into two proposals: the "operator overloading" part (likely non-contentious) and the "with operators from" part (which would likely be more opinionated).

@Haringat
Copy link
Author

Haringat commented Jan 5, 2023

@Pandapip1 that would solve this one case, but would create new problems. You could never define a class and use its operators in the same file, which sounds rather stupid, but would be necessary as you cannot work on classes before they are defined (so you cannot put the with statement before the class definition).
Also, I would consider imports to be the first statements in the files. And forcing with to come directly behind the imports (which are not even necessarily at the top of the file) would require really complex behavior definition.

@FrameMuse
Copy link

I think adding a new config option (like includeOperators) would solve this. So you either include all operators at once or not. Because choosing where to include operators still seems to be quite ambiguous.

To clarify, there should also be an option to allow at least creating operators (for library creators), so I'm talking about removing with at all.

@Haringat
Copy link
Author

Haringat commented Jan 6, 2023

@FrameMuse and how would you configure this?

@FrameMuse
Copy link

@FrameMuse and how would you configure this?

Config file?

@Haringat
Copy link
Author

Haringat commented Jan 6, 2023

@FrameMuse and what config file? A new convention for browsers? Sounds a bit overkill for a single JS feature to work.

@FrameMuse
Copy link

@FrameMuse and what config file? A new convention for browsers? Sounds a bit overkill for a single JS feature to work.

Yeah, didn't think about it.

@FrameMuse
Copy link

I still don't think with is a good solution, so there are two ways I see:

  • This feature is postponed for better times
  • The operators should be included with imported module

Because if not including them all at once, then we have big problems inventing new structures, which engine developers don't like very much (at least for this minor feature).

@Haringat
Copy link
Author

Haringat commented Jan 6, 2023

@FrameMuse I see a third solution: reimagine the proposal (although that would probably involve post-poning it). I have seen a few different approaches in the issues and came up with one myself.
But I agree that with is probably not a good idea.

@FrameMuse
Copy link

FrameMuse commented Jan 6, 2023

@Haringat I think you probably should create a new proposal since you have your own vision. This would be something like "define operator" as you suggested in this issue.

IMO, this is a nice approach, I like it.

@Pandapip1
Copy link

I too like the approach you linked. The syntax (at least to me) seems better than the with we have right now.

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

4 participants