-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[SVELTE] Transforms & remove Date.parse polyfill #4936
[SVELTE] Transforms & remove Date.parse polyfill #4936
Conversation
I like the idea, #4925 is waiting for you though. Lets get that one in, so we can look at this one (diff is rather large until that one lands). |
74aef8a
to
ca15b82
Compare
@@ -39,28 +38,22 @@ test('#deserialize', function(assert) { | |||
assert.equal(transform.deserialize(undefined), null); | |||
}); | |||
|
|||
test('#deserialize with different offset formats', function(assert) { | |||
testInDebug('#deserialize with different offset formats', function(assert) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should likely always test this, but only test the expect deprecation in debug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@runspired thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, but I'm not sure (based on how that stripping happens) how to set this up at a glance. I will think.
8af2bc1
to
0717f70
Compare
This PR branches off ofLanded :)svelte/rollup-private
and will need that to land first, the relevant commit is 89f4c9dThis PR brings Transforms back into the public folder (they had moved into the Private folder during the first phase of rollup).
The motivation for the original move was due to the location of initializers (later fixed in that PR) as well as the private
Date.parse
polyfill.In addition to bringing these back into the public folder, this removes the
Date.parse
polyfill as much as possible, adding a new deprecation for a corner case which our polyfill handled but which is not part of the spec.Removing the
Date.parse
polyfill has been debated before, here's a short TL;DROur custom Date parsing logic pre-dated and polyfilled the ES5 behavior; however, we now only support ES5 browsers.
In ES2015, initially there was a breaking change which prevented us from removing our custom parsing logic, however, that breaking change was reverted and not implemented by major browsers. We can now safely rely on the native
Date.parse
.Previous discussions of our own here: #2685
See: tc39/ecma262#138
and: tc39/ecma262#87
However, it turns our the polyfill we are using does not conform to the ECMA variant of ISO 8601, with a minor difference in that it allows for a "shorthand" timezone offset which is part of the greater spec but not ECMA.
This means that going immediately to the native
Date.parse
is not possible without a deprecation, which has been detected and added here: 74aef8a#diff-6da4887e7f026f285304a8bbb0f06e68R64