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

What would it mean to "decorate an export"? #135

Open
rbuckton opened this issue Aug 2, 2018 · 37 comments
Open

What would it mean to "decorate an export"? #135

rbuckton opened this issue Aug 2, 2018 · 37 comments

Comments

@rbuckton
Copy link
Collaborator

rbuckton commented Aug 2, 2018

This is a question often brought up in other issues (most recently in #69 (comment)) that might need to be better explored. The way I see it, there are a number of specific subtopics related to this:

  • If you could "decorate" any export, what would you expect to be able to do?
  • How would decorating an export affect the static semantics for import/export?
  • How would you apply a decorator to the following statements in a consistent and unambiguous way:
    • export * from "module";
    • export function F() {}
    • export class C {}
    • export default function() {}
    • export default class {}
    • export default expr;
    • export { x };
    • export { x as y };
  • Are there runtime semantics concerns with decorating exports?
  • Are there security concerns with decorating exports?
@rbuckton
Copy link
Collaborator Author

rbuckton commented Aug 2, 2018

  • If you could "decorate" any export, what would you expect to be able to do?

Some possible use cases (obviously not exhaustive):

  1. Rename the exported symbol.
  2. Remove the export.
  3. Make the export mutable on the import side.
  4. Wrap the export in a proxy/membrane.
  5. Perform security demands when imported.

(1) and (2) would have obvious effects on the static semantics for import/export. (3) would have effects on the runtime semantics of the module containing the export (as the local binding could change).

@doodadjs
Copy link

doodadjs commented Aug 2, 2018

By decorating an "export", what comes to me in mind is something like...

@allows("https://myapp.doodad.com")
export @static class A {....}

@ljharb
Copy link
Member

ljharb commented Aug 2, 2018

I could see it providing a "setter" and a "getter" - ie, when the live binding in the module changes, the "setter" would be able to wrap/replace the new export value, and when someone accesses the live binding, the "getter" would be called.

@doodadjs
Copy link

doodadjs commented Aug 2, 2018

Maybe also...

const foo = {count: 0}

@sharedAccrossRealms
export foo

@loganfsmyth
Copy link

Decorating exports seems to have a lot of the same questions around hoisting as decorating function declarations. When would the decorator on an export actually run?

@doodadjs
Copy link

doodadjs commented Aug 2, 2018

I'm afraid that, because of the static nature of "import/export", these decorators would be specific to the language, and non-expendable.

EDIT: And I think that's good.

@rbuckton
Copy link
Collaborator Author

rbuckton commented Aug 2, 2018

@doodadjs can you explain what you mean by "specific to the language, and non-expendable"?

@rbuckton
Copy link
Collaborator Author

rbuckton commented Aug 2, 2018

Decorating exports seems to have a lot of the same questions around hoisting as decorating function declarations. When would the decorator on an export actually run?

Decorators must be evaluated, which means they would have to follow the same evaluation semantics as the surrounding code so as not to have surprising effects for users.

It's not a big problem for a class declaration, since classes also require evaluation, however function declarations are hoisted. This could be observed in cases of a circular dependency between two modules. Decorating the export of a function declaration would put you in a very odd case where the local declaration is hoisted but the symbol on the import side is not initialized.

We would need to solve this problem for functions before we can solve it for exports, or such a feature would never be feasible.

@doodadjs
Copy link

doodadjs commented Aug 2, 2018

@rbuckton

can you explain what you mean by "specific to the language, and non-expendable"?

I mean just a closed set of them should exist, like other keywords in JS. But I tell that because I can't find how it could be evaluated otherwise. (I'm also thinking about bundlers.)

@doodadjs
Copy link

doodadjs commented Aug 2, 2018

There should be the concept of packages... I mean we bundle module files to a package, then import what that package exports, instead of combining everything into a single file. These packages could simply be tar files....

@rbuckton
Copy link
Collaborator Author

rbuckton commented Aug 3, 2018

can you explain what you mean by "specific to the language, and non-expendable"?

I mean just a closed set of them should exist, like other keywords in JS. But I tell that because I can't find how it could be evaluated otherwise. (I'm also thinking about bundlers.)

If it is a closed set with specific runtime behavior, then they probably should be keywords, not decorators. Decorators are intended to be used by developers to apply custom behavior in a declarative fashion.

@rbuckton
Copy link
Collaborator Author

rbuckton commented Aug 3, 2018

There should be the concept of packages... [...]

I'm not exactly certain I see how this applies to this topic. While "packages" are certainly a JS developer community concept, there still doesn't exist the notion of a "package" in ECMAScript. If possible, I'd like to keep this discussion focused on exports from a module.

@doodadjs
Copy link

doodadjs commented Aug 3, 2018

@rbuckton

If it is a closed set with specific runtime behavior, then they probably should be keywords, not decorators. Decorators are intended to be used by developers to apply custom behavior in a declarative fashion.

Cannot some @decorators be better provided by the runtime ? I think adding new keywords for the two examples I've presented there should not be considered. Like I told you, I'm actually worried about bundlers. What I propose then is to keep the "export" decorators possible for an incoming future.

If possible, I'd like to keep this discussion focused on exports from a module.

Yeah, good point. If at least I could know where to open that discussion, it would be great. Thanks.

@rbuckton
Copy link
Collaborator Author

rbuckton commented Aug 3, 2018

Cannot some @decorators be better provided by the runtime ?

I'm not opposed to the runtime providing decorators, and in fact encourage it. However, it would be much easier to use keywords for well-motivated behaviors that modify an export currently, due to the complexities of module evaluation. It would be inconsistent if developers could not write their own decorators against exports.

@doodadjs
Copy link

doodadjs commented Aug 4, 2018

@rbuckton

However, it would be much easier to use keywords for well-motivated behaviors that modify an export currently, due to the complexities of module evaluation.

Nope, introducing new keywords is extravagant now that we have decorators which could do that.

It would be inconsistent if developers could not write their own decorators against exports.

Probably, but if user-land decorators can be evaluated against "export" (and eventually "import"), that could break the static design of import/export. I have no objection to that though (that was my initial mental model of a "module", ie dynamic).

@rbuckton
Copy link
Collaborator Author

rbuckton commented Aug 6, 2018

@littledan: While I can understand tagging this as a "follow-on proposal", I feel we must explore this in some fashion before we can truly make a determination on #69, given that the possibility of "decorating an export" is one of the only real technical reasons that would motivate export coming first. As such, I would consider this also a Stage 3 blocker.

@littledan
Copy link
Member

Many of the arguments which advocates of export-before-decorators are using don't have to do with decorating exports ever being a thing in the future. I think we should consider this to block Stage 3 only if we come down to that one point being decisive.

@ljharb

This comment has been minimized.

@rbuckton
Copy link
Collaborator Author

rbuckton commented Aug 6, 2018

Many of the arguments which advocates of export-before-decorators are using don't have to do with decorating exports ever being a thing in the future. I think we should consider this to block Stage 3 only if we come down to that one point being decisive.

@waldemarhorwat specifically called out this behavior in #69 (comment) as a reason that "decorators before export" is not viable, so I believe that exploring this speaks directly to his concern.

@rbuckton
Copy link
Collaborator Author

rbuckton commented Aug 6, 2018

fwiw, I think decorators need to be included in toString even if they appear before export.

@ljharb: I don't believe that is relevant to this discussion, as #109 is the correct topic to discuss toString behavior.

@rbuckton
Copy link
Collaborator Author

rbuckton commented Aug 6, 2018

If you could "decorate" any export, what would you expect to be able to do?

I posted this in #69 (comment), but it is worth mentioning here: One option to avoid before/after ambiguity would be to have the "export descriptor" be part of the descriptor for the declaration:

function someExportDecorator(classDescriptor) {
  assert(classDescriptor.kind === "class");
  const savedGet = classDescriptor.export.get;
  classDescriptor.export.get = function () { return wrapWithMembrane(savedGet.call(this)); };
}

This would be somewhat symmetric with member decorators (aside from the property name):

function someMethodDecorator(memberDescriptor) {
  assert(memberDescriptor.kind === "method");
  const savedGet = memberDescriptor.descriptor.get;
  memberDescriptor.descriptor.get = function () { return wrapWithMembrane(savedGet.call(this)); };
}

If we ever support decorating other types of declarations, it would likely be useful to use the same decorator on different declaration kinds (i.e. "class", "function", etc.) and disambiguate behavior between them just as we can with members (i.e. "method", "field").

@maxnordlund
Copy link

Some here mentioned function hoisting and how that leads to circular dependencies. I suggest this should be seen as a TDZ, and possibly just throw a ReferenceError if trying to access those yet-to-be-imported bindings. These errors should be symmetric, so that actual evaluation order doesn't matter.


Two use cases that come to mind, if permitted, are:

1, Dynamically export everything from all files in a directory, kinda like require-dir. This implies export decorators are run before the import/export graph has been fully determined.
2, Compile some other language to JavaScript:

@template
export const = "<form>...</form>"

Of course this can be done using a normal function call, but a bundler may be able to use this for precompiling.

Both of these feels like a loader variant, which probably means they shouldn't be allowed. That begs the question, what other (valid) use cases are there?

Having a static import/export graph is really nice, and I assume people want to keep. But having access to property accessors would allow for some interesting things. Lazily evaluate exports for quicker startup, enforce security and support mutablity as mentioned above.

It also gives feature parity to CommonJS:

Object.defineProperty(exports, "foo", {
  get() {
    console.log("accessed module.foo")
    return 123
  }
})

@Jessidhia
Copy link

Your example of "Compile some other language to JavaScript" would be better done as a tagged template literal, not as a decorator.

For example, if you make it an export decorator, then you can only use it in an exported value. It would have to be a VariableDeclaration decorator to be able to use on non-exported values. But then it'd not be usable on expressions, only on VariableDeclaration; and all of the VariableDeclarator inside it would have to share the same set of decorators.

@maxnordlund
Copy link

To cite myself:

Of course this can be done using a normal function call, but a bundler may be able to use this for precompiling.

Tagged template literals can conceivably be precompiled by a bundler, if that bundler can guarantee that the symbol before the literal points to a known tagging function. I.e. this.foo in

class Tag {
  method() { return this.foo`bar ${baz}` }
}

That is not would I expect when reading code like the above, tagged literals are evaluated when the VM hits that code path. Which allows for dynamic choices, like the above. On the other hand we have decorators. That signal that this is done once, at definition time, which may be inlined by the compiler/bundler (take a look at babel's output for legacy decorators).

Here we're discussing what an export decorator means, if anything, and here's the million dollar question: When should an export decorator be run? Before or after the module graph is done?

@ljharb
Copy link
Member

ljharb commented Oct 26, 2018

Since the decorator itself would have to be imported from an evaluated module, or parsed in that file - and both of those things happen after the module graph is "done" - it would have to run after, which would be too late to modify the module graph.

@maxnordlund
Copy link

I guess then it's a matter of allowing recursion or not. That, or back to builtin decorators/syntax, for use cases like getter/setters.

@robbiespeed
Copy link
Contributor

I'd argue that if exports ever can be decorated, regardless of what those decorators do the syntax should be more explicit.

@foo export
@bar
class Baz () {}

It's unclear whats happening here, the export could very easily be missed.

Examples of clearer syntax:

@export:foo export
@bar
class Baz () {}
@export(foo) export
@bar
class Baz () {}

Either way if there's some sort of consensus that export decorators would need to have a more explicit syntax, then I think we could consider this a non issue for resolving #69.

@jsg2021
Copy link

jsg2021 commented Oct 26, 2018

Given the static constraints of the graph, I would suggest we consider decoration of exports as not viable, and give up this idea.

We can push the decoration into the module itself and perform your membrane/accessor stuff on the thing being exported.

@david0178418
Copy link

I think @jsg2021 has the only correct answer here. It feels like this is attempting to describe something that would require an entirely different construct that would also have to statically evaluatable.

@maxnordlund
Copy link

Indeed, and the other use cases will have to be solved in some other way if at all.

@dongryphon
Copy link

I would greatly prefer the @dec export class X (or @dec export default class X) form and, therefore, very much agree with both of these sentiments:

Given the static constraints of the graph, I would suggest we consider decoration of exports as not viable, and give up this idea.

I'd argue that if exports ever can be decorated, regardless of what those decorators do the syntax should be more explicit. ...snip... Either way if there's some sort of consensus that export decorators would need to have a more explicit syntax, then I think we could consider this a non issue

@david0178418
Copy link

david0178418 commented Nov 5, 2018

Is there movement on this as a standard? Last I saw was a vote a few of weeks ago with a slight lean towards export before decorator. If this has been no movement and this is still being hung up, I'd propose disallowing either until this gets worked out. How to export a decorated class is peripheral to the core intent of decorators. Otherwise, we're stuck in an infinite loop.

EDIT: Actually, this comment doesn't belong here. It belongs on #69. Reposting to the more appropriate ticket

@littledan
Copy link
Member

littledan commented Jan 8, 2019

I've been thinking about this question more, and I'm coming to the conclusion that we should really put decorators immediately the construct that they're modifying. I can't think of a good reason to write an procedural decorator for exports, but I wonder if we should be using decorator-like syntax for other sorts of modifications of things. I'm working on a document and TC39 presentation to explain more. We can use constructs to have a scope attached to the decorator, but given how much nesting and composition JavaScript allows, it seems awkward to make too much use of this to target sub-expressionsdeclarations, and focus instead on using these prefixes for different semantic purposes. For this sort of future-proofing reason, I'd like to conclude that export should come before decorators.

@dantman
Copy link

dantman commented Jan 8, 2019

to target sub-expressions

You mean sub-declarations.

@littledan
Copy link
Member

@dantman Yes, you're exactly right. I don't see decorating arbitrary expressions being syntactically viable (but this proposal includes decorating class expressions, and I could see decorating function expressions and object literals in the future).

@pzuraq
Copy link
Collaborator

pzuraq commented Jan 8, 2019

FWIW at this point I think it's most important that decorators move forward in the process, and get wider adoption and usage. I definitely don't think it's worth gating on this discussion any longer, and support the conclusion that TC39 lands on 100% - whatever it is, the JS ecosystem will figure it out and get used to it. The polls and discussion show that this really is a matter of aesthetic preference (which is why it is being bikeshedded to death I think).

Also, what I really wanted to say here was a massive thank you to @littledan, @rbuckton, and the other TC39 members for continuing to push the spec forward and dealing with these long (and extremely pedantic) discussions! In the end I think y'all are doing amazing work and I personally fully support whatever conclusions you come to 😄

@jsg2021
Copy link

jsg2021 commented Jan 8, 2019

but I wonder if we should be using decorator-like syntax for other sorts of modifications of things.

I'd be in favor of a decorator-like export syntax :)

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