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

Target es5 when compiling JS #67

Merged
merged 1 commit into from
Aug 2, 2020

Conversation

jamesmoss
Copy link
Contributor

This fixes #52 by targeting ES5 instead of ES6 when compiling. I've included the DOM and ES2015 libs so that types can be correctly checked.

I ran into issues with ts-jest failing tests with ES5 so I've setup a new tsconfig.test.json file which sets the target back to ES6 in the test environment.

@jharrilim
Copy link
Collaborator

Looks good, thank you @jamesmoss!

@jharrilim jharrilim merged commit d11364e into rehooks:master Aug 2, 2020
@jamesmoss
Copy link
Contributor Author

@jharrilim Apologies but you're going to need to revert this as there's currently an issue which causes the lib to break when writing to the storage. Unfortunately the IE11 compatibility issue is a lot more complex than I first though.

Typescript can't handle transpiling classes which extend built in classes to ES5 (see this open issue microsoft/TypeScript#15397). As LocalStorageChanged extends CustomEvent in local-storage-events.ts this means that the ES5 version will never work properly.

Right now the only fix I can see is to get rid of LocalStorageChanged and just use CustomEvent directly. I had a go at this and got it working but my Typescript's not too hot so I'm not happy submitting a PR.

@jharrilim
Copy link
Collaborator

Sure, we can make a fix so that it creates an instance of CustomEvent instead of a subclass of CustomEvent.

I have just tried

export function LocalStorageChanged<TValue>(payload: LocalStorageEventPayload<TValue>) {
    return new CustomEvent('onLocalStorageChange', { detail: payload });
}
LocalStorageChanged.eventName = 'onLocalStorageChange';

and it currently generates this ES5:

function LocalStorageChanged(payload) {
    return new CustomEvent('onLocalStorageChange', { detail: payload });
}
exports.LocalStorageChanged = LocalStorageChanged;
LocalStorageChanged.eventName = 'onLocalStorageChange';

I was against it at first, but I think we should also add the CustomEvent polyfill shown here too:
https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent/CustomEvent#Polyfill

@jharrilim
Copy link
Collaborator

Made an update here which no longer uses inheritance, and contains a CustomEvent polyfill:

https://github.com/rehooks/local-storage/tree/patch-67

If you have time, please try this out as well. I think it should work.

@InSuperposition
Copy link

Any update on releasing this patch?

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.

Should it work in IE11?
3 participants