Update Class Fields to Stage 3 and change default behavior by noahlemen · Pull Request #6076 · babel/babel

noahlemen · 2017-08-09T03:25:03Z

Q	A
Patch: Bug Fix?
Major: Breaking Change?	✔️ (change to transform option)
Minor: New Feature?
Deprecations?
Spec Compliancy?	✔️
Tests Added/Pass?
Fixed Tickets	Fixes #6073
License	MIT
Doc PR
Dependency Changes

moves transform-class-properties from stage 2 to 3.
changes behavior to use spec mode by default, as requested by @hzoo in Update Class Fields to Stage 3 and change default behavior #6073 (please let me know if i misunderstood any part of that!)
updates tests
updates readme

noahlemen · 2017-08-09T03:25:50Z

i added the {"loose": true} option to tests that were previously defaulting to non-spec mode. would it be better to update those tests' expected.js to use spec mode's output instead?

noahlemen · 2017-08-09T03:26:22Z

packages/babel-plugin-transform-class-properties/README.md

 `boolean`, defaults to `false`.

-Class properties are compiled to use `Object.defineProperty`. Static fields are now defined even if they are not initialized.
+Class properties are compiled use an assignment expression instead of `Object.defineProperty`. Static fields are now not defined if they are not initialized.


not sure how confident i am of the language here. does this capture the intended behavior?

We can put the code example here to be more clear.

so an example of the defineProperty vs. this.x = a and also this.x = undefined

added an example, though it may be a bit lengthy. let me know if you think we can trim it down a bit!

Jessidhia · 2017-08-09T07:00:14Z

packages/babel-plugin-transform-class-properties/src/index.js


-          // In non-spec mode, all properties without values are ignored.
+          // In loose mode, all properties without values are ignored.
          // In spec mode, *static* properties without values are still defined (see below).


IIUC, in spec mode, all properties should be defined. This may also be a new change on the stage-3 version.

https://tc39.github.io/proposal-class-fields/#runtime-semantics-class-field-definition-evaluation the Record is still created even if the Initializer is missing

https://tc39.github.io/proposal-class-fields/#initialize-public-instance-fields InitializeInstanceFields does not care whether the record has an initializer

https://tc39.github.io/proposal-class-fields/#sec-define-field DefineField will use an initValue of undefined if there is no initializer

yep, looks like it to me too! i'll change that here.

think i covered these now. if you've got a chance to take another look, let me know if anything is missing!

Jessidhia · 2017-08-09T07:01:32Z

packages/babel-plugin-transform-class-properties/src/index.js

@@ -36,7 +36,7 @@ export default function({ types: t }) {
      VALUE: value ? value : t.identifier("undefined"),


Probably better to use t.unaryExpression('void', t.numericLiteral(0)) to avoid any potential shadowing issues (fix out of scope maybe?)

we have a method for it path.scope.buildUndefinedNode( - btw this should be a linting rule in babel/run against babel?

👍 i'll replace with buildUndefinedNode here

@hzoo regarding linting, would the rule be to always disallow t.identifier("undefined") in favor of path.scope.buildUndefinedNode()?

I think so at least for us (that can be another pr, can do an inline plugin in the babelrc.js if we wanted), not sure the best way

gotcha. cool, maybe i'll take a swing at that after this PR

jridgewell · 2017-08-09T19:02:24Z

packages/babel-plugin-transform-class-properties/README.md

 `boolean`, defaults to `false`.

-Class properties are compiled to use `Object.defineProperty`. Static fields are now defined even if they are not initialized.
+Class properties are compiled use an assignment expression instead of `Object.defineProperty`. Static fields are now not defined if they are not initialized.


Why are static fields not initialized? They should be undefined, too.

i think it was never added for the default mode (which is now changing to loose)

yeah, i just kept the behavior that existed as default for the new "loose" mode -- should this be changed?

…ior to always define both static and nonstatic class properties regardless of spec/loose mode; update tests

hzoo · 2017-08-10T15:09:14Z

Amazing work @kedromelon!

noahlemen · 2017-08-10T15:10:22Z

😄 thanks for the help everyone!

hzoo · 2017-08-10T15:14:45Z

Was a pretty big refactor! Also unforunately all the PRs will conflict since we are changing 3 different plugins to stage 2/3

hzoo · 2017-08-10T15:20:29Z

@kedromelon also I noticed you were branching off of 7.0? I would suggest making a new branch for each PR in case you have multiple, and easier to manage

noahlemen · 2017-08-10T15:22:06Z

@hzoo yeah, i probably will do that in the future! just didn't bother yet since i haven't had more than one PR going

add transform-class-properties to stage 3, set spec mode to default

ce38e65

noahlemen commented Aug 9, 2017

View reviewed changes

Jessidhia reviewed Aug 9, 2017

View reviewed changes

hzoo added the PR: Spec Compliance 👓 A type of pull request used for our changelog categories label Aug 9, 2017

jridgewell requested changes Aug 9, 2017

View reviewed changes

update readme with examples; use buildUndefinedNode(); change behav…

efc8321

…ior to always define both static and nonstatic class properties regardless of spec/loose mode; update tests

noahlemen force-pushed the 7.0 branch from 428cdff to efc8321 Compare August 10, 2017 01:35

jridgewell approved these changes Aug 10, 2017

View reviewed changes

existentialism approved these changes Aug 10, 2017

View reviewed changes

hzoo added the Priority: High label Aug 10, 2017

Merge branch '7.0' into 7.0

8d62f28

Update index.js

a0ef70c

hzoo merged commit 4fdd756 into babel:7.0 Aug 10, 2017

hzoo mentioned this pull request Aug 10, 2017

Split export extensions into 2 #6080

Merged

noahlemen mentioned this pull request Aug 10, 2017

transform-class-properties: treatment of undefined properties (i.e. properties without default values) #5056

Closed

hzoo mentioned this pull request Aug 10, 2017

Changed transform-class-properties to declare uninitialized properties #5115

Closed

existentialism mentioned this pull request Aug 11, 2017

Fix class prop test fixture #6090

Merged

Jessidhia referenced this pull request Aug 12, 2017

Update outdated test fixture

cd66657

This was referenced Aug 12, 2017

linting: disallow t.identifier("undefined") in plugins #6096

Merged

default to spec mode for template literal transform #6098

Merged

hzoo mentioned this pull request Aug 15, 2017

Create aug-16.md babel/notes#32

Merged

chicoxyzzy mentioned this pull request Aug 21, 2017

Move class fields to stage 3 babel/proposals#25

Merged

noahlemen mentioned this pull request Aug 30, 2017

update transform-es2015-arrow-functions to default to spec mode #6170

Closed

3 tasks

loganfsmyth mentioned this pull request Aug 31, 2017

Babel 7 alpha 20 break decorators with class properties. #6181

Closed

noahlemen mentioned this pull request Sep 3, 2017

warn about class properties support in decorators readme and error messaging #6196

Merged

hzoo mentioned this pull request Sep 12, 2017

July 2017 babel/proposals#19

Closed

10 tasks

lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 6, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 6, 2019

		@@ -36,7 +36,7 @@ export default function({ types: t }) {
		VALUE: value ? value : t.identifier("undefined"),

Uh oh!

Conversation

noahlemen commented Aug 9, 2017 • edited by hzoo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

noahlemen commented Aug 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jessidhia Aug 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hzoo Aug 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hzoo commented Aug 10, 2017

Uh oh!

noahlemen commented Aug 10, 2017

Uh oh!

hzoo commented Aug 10, 2017

Uh oh!

hzoo commented Aug 10, 2017

Uh oh!

noahlemen commented Aug 10, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

noahlemen commented Aug 9, 2017 •

edited by hzoo

Loading

noahlemen commented Aug 9, 2017 •

edited

Loading

Jessidhia Aug 9, 2017 •

edited

Loading

hzoo Aug 9, 2017 •

edited

Loading