-
Notifications
You must be signed in to change notification settings - Fork 556
feat(chat): include in UMD build #6793
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
base: master
Are you sure you want to change the base?
Conversation
Did not remove the umd file yet, as it's used by vue instantsearch and algolia-experiences bundles, which didn't get a change in setup.
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit b7e1292:
|
|
Got it to work and tested on |
|
That's a much larger impact than I expected for chat. Is it all because of the dependencies or also because the chat widget/connector uses a lot of local code? |
| default as defaultConstructor, | ||
| } from 'algoliasearch/lite'; | ||
| import instantsearch from '../../src'; | ||
| import type instantsearch from '../../src'; |
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.
instantsearch is used as more than a type, maybe the import needs to be the umd version for storybook to avoid chat?
c1fa2e8 to
9af869e
Compare
549bff4 to
232cc10
Compare
232cc10 to
b7e1292
Compare
| export const ZodFirstPartyTypeKind = {}; | ||
| export const toJSONSchema = {}; | ||
| export const safeParseAsync = {}; | ||
| export const z = new Proxy( |
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.
at which point is this evaluated? do we need to check if proxy exists first in case it gets evaluated in IE11?
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'm ok with chat not working in IE11, but just ensuring that the entire InstantSearch doesn't throw
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.
Yeah the constructor does get called directly. There are polyfills though if we still actually care about the tests passing ?
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.
We may need to do typeof Proxy === 'object' ? new Proxy : null or similar to at least not have a global error thrown then? not sure how to check tbh, just asking
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.
It would still throw though cause zod schemas are created when the script is evaluated... so we'd have to define all those functions just for IE11 to work and make the bundle bigger
Haroenv
left a comment
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.
✅ but I can't approve it as I initially opened the PR
Did not remove the umd file yet, as it's used by vue instantsearch and algolia-experiences bundles, which didn't get a change in setup.
Summary
Result