[Fix] Handle special case for localhost in development environment in isThirdwebUrl function #3369

jnsdls · 2024-06-18T00:03:03Z

TL;DR

This PR updates the isThirdwebUrl function to properly handle the localhost hostname in the development environment.

What changed?

Added a check to see if process.env.NODE_ENV is set to development
If in development, it checks if the hostname is localhost and treats it as a Thirdweb URL

How to test?

Ensure the application runs in both development and production environments
In development, verify URLs with localhost return true for isThirdwebUrl
In production, verify URLs with localhost return false for isThirdwebUrl

Why make this change?

This change was made to correctly classify localhost as a Thirdweb URL when running in a development environment, allowing for better development workflow and environment parity.

PR-Codex overview

The focus of this PR is to exclude localhost from being considered a thirdweb URL, except in development mode.

Detailed summary

Excludes localhost from thirdweb URL check in fetch.ts except in development mode.
Adds special handling for localhost in development environment.
Improves accuracy of identifying thirdweb URLs.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

vercel · 2024-06-18T00:03:05Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
thirdweb_playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 18, 2024 0:16am

changeset-bot · 2024-06-18T00:03:08Z

🦋 Changeset detected

Latest commit: b808652

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages

Name	Type
thirdweb	Patch
@thirdweb-dev/sdk	Patch
@thirdweb-dev/cli	Patch
@thirdweb-dev/react-core	Patch
@thirdweb-dev/react	Patch
@thirdweb-dev/unity-js-bridge	Patch
@thirdweb-dev/wallets	Patch
@thirdweb-dev/auth	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

graphite-app · 2024-06-18T00:03:09Z

Your org requires the Graphite merge queue for merging into main

Add the label “merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

jnsdls · 2024-06-18T00:03:17Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @jnsdls and the rest of your teammates on Graphite

codspeed-hq · 2024-06-18T00:06:59Z

CodSpeed Performance Report

Merging #3369 will improve performances by 10.6%

_{Comparing fix-thirdweb-url-local-dev (b808652) with main (c42a34f)}

Summary

⚡ 1 improvements
✅ 8 untouched benchmarks

Benchmarks breakdown

	Benchmark	`main`	`fix-thirdweb-url-local-dev`	Change
⚡	`keccakId`	1.4 ms	1.3 ms	+10.6%

github-actions · 2024-06-18T00:07:39Z

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
thirdweb (esm)	40.6 KB (0%)	813 ms (0%)	506 ms (-11.58% 🔽)	1.4 s
thirdweb (cjs)	90.26 KB (0%)	1.9 s (0%)	1.1 s (-33.41% 🔽)	3 s
thirdweb (minimal + tree-shaking)	4.74 KB (0%)	95 ms (0%)	46 ms (+9.93% 🔺)	141 ms
thirdweb/chains (tree-shaking)	423 B (0%)	10 ms (0%)	16 ms (-85.96% 🔽)	26 ms
thirdweb/react (minimal + tree-shaking)	13.45 KB (0%)	270 ms (0%)	82 ms (+21.83% 🔺)	351 ms

codecov · 2024-06-18T00:10:24Z

Codecov Report

Attention: Patch coverage is 41.66667% with 7 lines in your changes missing coverage. Please review.

Project coverage is 62.89%. Comparing base (c42a34f) to head (59c70ea).

❗ Current head 59c70ea differs from pull request most recent head b808652

Please upload reports for the commit b808652 to get more accurate results.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3369      +/-   ##
==========================================
+ Coverage   62.85%   62.89%   +0.04%     
==========================================
  Files         862      861       -1     
  Lines       63908    63843      -65     
  Branches     3462     3463       +1     
==========================================
- Hits        40168    40156      -12     
+ Misses      23063    23009      -54     
- Partials      677      678       +1

Flag	Coverage Δ		*Carryforward flag
legacy_packages	`65.60% <ø> (ø)`		Carriedforward from 9dd1b5a
packages	`62.33% <41.66%> (+0.05%)`	⬆️

*This pull request uses carry forward flags. Click here to find out more.

Files	Coverage Δ
packages/thirdweb/src/utils/fetch.ts	`82.01% <41.66%> (-2.99%)`	⬇️

... and 1 file with indirect coverage changes

graphite-app · 2024-06-18T00:12:14Z

Merge activity

Jun 17, 8:12 PM EDT: jnsdls added this pull request to the Graphite merge queue.
Jun 17, 8:14 PM EDT: jnsdls merged this pull request with the Graphite merge queue.

jnsdls · 2024-06-18T00:12:30Z

fixes: CNCT-1370

… isThirdwebUrl function (#3369) ### TL;DR This PR updates the `isThirdwebUrl` function to properly handle the `localhost` hostname in the development environment. ### What changed? - Added a check to see if `process.env.NODE_ENV` is set to `development` - If in development, it checks if the hostname is `localhost` and treats it as a Thirdweb URL ### How to test? - Ensure the application runs in both development and production environments - In development, verify URLs with `localhost` return `true` for `isThirdwebUrl` - In production, verify URLs with `localhost` return `false` for `isThirdwebUrl` ### Why make this change? This change was made to correctly classify `localhost` as a Thirdweb URL when running in a development environment, allowing for better development workflow and environment parity. ---  --- ## PR-Codex overview The focus of this PR is to exclude localhost from being considered a thirdweb URL, except in development mode. ### Detailed summary - Excludes localhost from thirdweb URL check in `fetch.ts` except in development mode. - Adds special handling for localhost in development environment. - Improves accuracy of identifying thirdweb URLs. > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`

linear · 2024-06-18T00:13:31Z

CNCT-1370 Cannot use custom bundlers on local network for A.A with Connect SDK

jnsdls marked this pull request as ready for review June 18, 2024 00:03

jnsdls force-pushed the fix-thirdweb-url-local-dev branch from 7d6f4e4 to 59c70ea Compare June 18, 2024 00:04

vercel bot deployed to Preview June 18, 2024 00:07 View deployment

jnsdls requested a review from joaquim-verges June 18, 2024 00:10

joaquim-verges approved these changes Jun 18, 2024

View reviewed changes

jnsdls force-pushed the fix-thirdweb-url-local-dev branch from 59c70ea to b808652 Compare June 18, 2024 00:13

graphite-app bot merged commit b808652 into main Jun 18, 2024
20 of 21 checks passed

graphite-app bot deleted the fix-thirdweb-url-local-dev branch June 18, 2024 00:14

vercel bot deployed to Production June 18, 2024 00:16 View deployment

vercel bot deployed to Preview June 18, 2024 00:16 View deployment

jnsdls mentioned this pull request Jun 18, 2024

Version Packages #3367

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix] Handle special case for localhost in development environment in isThirdwebUrl function #3369

[Fix] Handle special case for localhost in development environment in isThirdwebUrl function #3369

jnsdls commented Jun 18, 2024 •

edited

Loading

vercel bot commented Jun 18, 2024 •

edited

Loading

changeset-bot bot commented Jun 18, 2024 •

edited

Loading

graphite-app bot commented Jun 18, 2024

jnsdls commented Jun 18, 2024

codspeed-hq bot commented Jun 18, 2024 •

edited

Loading

github-actions bot commented Jun 18, 2024 •

edited

Loading

codecov bot commented Jun 18, 2024 •

edited

Loading

graphite-app bot commented Jun 18, 2024 •

edited

Loading

jnsdls commented Jun 18, 2024

linear bot commented Jun 18, 2024

[Fix] Handle special case for localhost in development environment in isThirdwebUrl function #3369

[Fix] Handle special case for localhost in development environment in isThirdwebUrl function #3369

Conversation

jnsdls commented Jun 18, 2024 • edited Loading

TL;DR

What changed?

How to test?

Why make this change?

PR-Codex overview

Detailed summary

vercel bot commented Jun 18, 2024 • edited Loading

changeset-bot bot commented Jun 18, 2024 • edited Loading

🦋 Changeset detected

graphite-app bot commented Jun 18, 2024

Your org requires the Graphite merge queue for merging into main

jnsdls commented Jun 18, 2024

codspeed-hq bot commented Jun 18, 2024 • edited Loading

CodSpeed Performance Report

Merging #3369 will improve performances by 10.6%

Summary

Benchmarks breakdown

github-actions bot commented Jun 18, 2024 • edited Loading

size-limit report 📦

codecov bot commented Jun 18, 2024 • edited Loading

Codecov Report

graphite-app bot commented Jun 18, 2024 • edited Loading

Merge activity

jnsdls commented Jun 18, 2024

linear bot commented Jun 18, 2024

jnsdls commented Jun 18, 2024 •

edited

Loading

vercel bot commented Jun 18, 2024 •

edited

Loading

changeset-bot bot commented Jun 18, 2024 •

edited

Loading

codspeed-hq bot commented Jun 18, 2024 •

edited

Loading

github-actions bot commented Jun 18, 2024 •

edited

Loading

codecov bot commented Jun 18, 2024 •

edited

Loading

graphite-app bot commented Jun 18, 2024 •

edited

Loading