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

fix(integration): Don't mangle localforage internals #5534

Merged
merged 2 commits into from Aug 8, 2022

Conversation

lforst
Copy link
Member

@lforst lforst commented Aug 8, 2022

The localforage library (used in offline integration) has some internal fields with a leading underscore (e.g. _initStorage). In our building process, we allow terser to mangle fields with a leading underscore to reduce bundle size when using classes with private fields. This causes internal breakage within the localforage package and throwing a "Error: Custom driver not compliant" error.

This PR marks some localforage-internal fields as reserved so they don't get mangled. The fields are taken from the localforage docs by searching the page for "_".

Fixes #5527

@lforst lforst self-assigned this Aug 8, 2022
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

Nice fix! I wonder if there's any possibility to mark these field names as reserved only in the localforage "context" (if that makes sense)? But then again, I've never come across these field names in our project so I guess it's not a big issue(?)

@lforst
Copy link
Member Author

lforst commented Aug 8, 2022

I wonder if there's any possibility to mark these field names as reserved only in the localforage "context" (if that makes sense)?

@Lms24 Yeah I thought about the same thing. I looked in the terser settings if there is some way to apply mangling settings to some isolated context but I don't think there is. :(

Merging this but hopefully, we can get rid of classes in the SDK so we don't need to mangle private fields at all anymore.

@lforst lforst merged commit 36c938c into master Aug 8, 2022
@lforst lforst deleted the lforst-localforage-mangling-fix branch August 8, 2022 10:47
@Lms24
Copy link
Member

Lms24 commented Aug 8, 2022

Yeah, I think it's totally fine for the time being!

we can get rid of classes in the SDK

A dev can always dream ;)

@lobsterkatie
Copy link
Member

If we're going to get rid of anything, it's our localforage dependency. It's given us almost nothing but grief from the time we added it, and all it does is wrap native APIs. Someday, when we have time...

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.

Error: Custom driver not compliant when using minified offline integration
3 participants