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

[SVELTE] Transforms & remove Date.parse polyfill #4936

Merged
merged 5 commits into from
Apr 26, 2017

Conversation

runspired
Copy link
Contributor

@runspired runspired commented Apr 22, 2017

This PR branches off of svelte/rollup-private and will need that to land first, the relevant commit is 89f4c9d Landed :)

  • This 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;DR

Our 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

@runspired runspired changed the title Svelte/cleanup transforms [SVELTE] Transforms & Date Apr 22, 2017
@runspired runspired changed the title [SVELTE] Transforms & Date [SVELTE] Transforms & remove Date.parse polyfill Apr 22, 2017
@stefanpenner
Copy link
Member

stefanpenner commented Apr 24, 2017

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).

@@ -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) {
Copy link
Member

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.

Copy link
Member

@stefanpenner stefanpenner Apr 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@runspired thoughts?

Copy link
Contributor Author

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.

@stefanpenner stefanpenner merged commit 05e9528 into emberjs:master Apr 26, 2017
@stefanpenner stefanpenner deleted the svelte/cleanup-transforms branch April 26, 2017 20:05
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

Successfully merging this pull request may close these issues.

2 participants