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(javascript): dependency imports and low-level init #3596

Merged
merged 8 commits into from
Aug 28, 2024

Conversation

shortcuts
Copy link
Member

@shortcuts shortcuts commented Aug 25, 2024

🧭 What and Why

🎟 JIRA Ticket: https://algolia.atlassian.net/browse/DI-2828

Changes included:

closes algolia/algoliasearch-client-javascript#1538

we import from the src of the dependency which is fine in the monorepo context, but the path isn't correct when importing from the built package

we actually don't need the low level init method, we can just use the regular instance and spread it, it should behave the same

@shortcuts shortcuts self-assigned this Aug 25, 2024
@algolia-bot
Copy link
Collaborator

algolia-bot commented Aug 25, 2024

✔️ Code generated!

Name Link
🪓 Triggered by 17f1c049bf02781e6ac70a08b89da0d99584ee32
🍃 Generated commit d9cc222008779e5595186c84d85a555c2133a987
🌲 Generated branch generated/fix/javascript-algoliasearch-build
📊 Benchmark results

Benchmarks performed on the method using a mock server, the results might not reflect the real-world performance.

Language Req/s
javascript 1694

@shortcuts shortcuts marked this pull request as ready for review August 25, 2024 16:59
@shortcuts shortcuts requested a review from a team as a code owner August 25, 2024 16:59
@@ -276,6 +276,7 @@ export function buildConfigs(pkg) {
}),
nodeResolve({
preferBuiltins: true,
browser: isUmdBuild || isEsmBrowserBuild,
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

ok it's not fully working

...initOptions.options,
...initOptions,
});
return recommendClient(initOptions.appId || appId, initOptions.apiKey ||apiKey, initOptions.options);
Copy link
Member Author

Choose a reason for hiding this comment

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

this is redundant and I think was legacy, now that standalone clients checks it, we don't need it here

@shortcuts shortcuts marked this pull request as draft August 25, 2024 17:22
@shortcuts shortcuts force-pushed the fix/javascript-algoliasearch-build branch from d953385 to 85dec20 Compare August 28, 2024 07:46
@shortcuts shortcuts changed the base branch from main to fix/partial-update August 28, 2024 07:46
Base automatically changed from fix/partial-update to main August 28, 2024 10:02
@shortcuts shortcuts force-pushed the fix/javascript-algoliasearch-build branch from 85dec20 to 5204d8d Compare August 28, 2024 12:06
@shortcuts shortcuts force-pushed the fix/javascript-algoliasearch-build branch from 5204d8d to 16c752d Compare August 28, 2024 12:06
Copy link

github-actions bot commented Aug 28, 2024

@shortcuts shortcuts marked this pull request as ready for review August 28, 2024 13:27
Copy link
Collaborator

@millotp millotp left a comment

Choose a reason for hiding this comment

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

looks good

@shortcuts shortcuts enabled auto-merge (squash) August 28, 2024 22:11
@shortcuts shortcuts merged commit 7189cbe into main Aug 28, 2024
11 checks passed
@shortcuts shortcuts deleted the fix/javascript-algoliasearch-build branch August 28, 2024 22:24
algolia-bot added a commit that referenced this pull request Aug 28, 2024
algolia-bot added a commit to algolia/algoliasearch-client-javascript that referenced this pull request Aug 28, 2024
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.

algoliasearch v5 not building in a typescript project
3 participants