GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Allow decorators in ES3 by rbuckton · Pull Request #4741 · microsoft/TypeScript

Conversation

@rbuckton

@rbuckton Ron Buckton (rbuckton) commented Sep 11, 2015

Copy link
Copy Markdown
Contributor

This change modifies the checker and the emitter to support the use of decorators when targeting ES3. In addition to removing the ES5-specific use of reduceRight from the __decorate helper, this change also inlines the calls to Object.getOwnPropertyDescriptor and Object.defineProperty in a backwards-compatible fashion that allows us to clean up the emit for ES5 and later by removing various repetitive calls to the aforementioned Object methods.

Addresses #4681.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@rbuckton
Allow decorators in ES3
7610048
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-not-required label Sep 11, 2015
@DanielRosenwasser

Daniel Rosenwasser (DanielRosenwasser) commented Sep 11, 2015

Copy link
Copy Markdown
Member

I'm not sure if I'm okay with dropping the error message - I think that decorators that won't properly work in ES3 should get an error.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@rbuckton
Updated baselines
c46309d
@rbuckton

Ron Buckton (rbuckton) commented Sep 11, 2015

Copy link
Copy Markdown
Contributor Author

Daniel Rosenwasser (@DanielRosenwasser) Are you talking about a specific error? Part of this change is to have the emit for method decorators in ES3 not include the property descriptor, just as with decorators on property declarations.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mhegazy

Mohamed Hegazy (mhegazy) commented Sep 11, 2015

Copy link
Copy Markdown
Contributor

but this will have a different behavior for ES3/ES5 for method decorators with no errors.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@DanielRosenwasser

Daniel Rosenwasser (DanielRosenwasser) commented Sep 11, 2015

Copy link
Copy Markdown
Member

Yes, if method decorators won't work properly, then you should warn users that method decorators are only supported in ES5 or higher.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@rbuckton

Ron Buckton (rbuckton) commented Sep 21, 2015

Copy link
Copy Markdown
Contributor Author

Daniel Rosenwasser (@DanielRosenwasser), In this change method decorators will work in the same way that decorators for property declarations work. No descriptor will be provided nor accepted. There will be no error.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Ron Buckton (rbuckton) added 2 commits October 1, 2015 16:56
@rbuckton
Merge branch 'master' into es3Decorators
be6e43e
@rbuckton
Fix linter errors
8a41cde
@rbuckton

Ron Buckton (rbuckton) commented Oct 2, 2015

Copy link
Copy Markdown
Contributor Author

Daniel Rosenwasser (@DanielRosenwasser), Mohamed Hegazy (@mhegazy) Any further concerns/discussion with this PR?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mhegazy

Mohamed Hegazy (mhegazy) commented Oct 2, 2015

Copy link
Copy Markdown
Contributor

👍
I am still not sure about the error though
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@rbuckton

Ron Buckton (rbuckton) commented Oct 2, 2015

Copy link
Copy Markdown
Contributor Author

The error that I removed no longer applies. The only difference between decorators in ES3 and ES5 is that ES3 decorators cannot take and don't observe a descriptor.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@DanielRosenwasser

Daniel Rosenwasser (DanielRosenwasser) commented Oct 2, 2015

Copy link
Copy Markdown
Member

Ron Buckton (@rbuckton) I'll need to ask a few questions about this offline.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Ron Buckton (rbuckton) added a commit that referenced this pull request Oct 2, 2015
@rbuckton
Merge pull request #4741 from Microsoft/es3Decorators
331d26f 
Allow decorators in ES3
@rbuckton
Ron Buckton (rbuckton) merged commit 331d26f into master Oct 2, 2015
@rbuckton
Ron Buckton (rbuckton) deleted the es3Decorators branch October 2, 2015 23:22
@rbuckton Ron Buckton (rbuckton) mentioned this pull request Oct 2, 2015
Partial support decorators when targeting ES3 #4681
Closed
@microsoft Microsoft (microsoft) locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@rbuckton @DanielRosenwasser @mhegazy @msftclas
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.