feat(remix): Add Remix 2.x release support. - #8940

Merged
AbhiPrasad merged 15 commits into
developfrom
onur/remix-v2-e2e-tests
Sep 19, 2023
Merged

feat(remix): Add Remix 2.x release support.#8940
AbhiPrasad merged 15 commits into
developfrom
onur/remix-v2-e2e-tests

Conversation

@onurtemizkan

@onurtemizkanonurtemizkan commented Sep 4, 2023•
edited by Lms24
Loading

Copy link
Copy Markdown
Contributor

Resolves: #8681
closes#9040

Summary:

  • Updated Node.JS version used on E2E tests to 18.x. (This is required for Remix v2.x, and 18.x is the current LTS so updated this for all) chore(e2e-tests): Use Node 18 for E2E tests. #8964
  • Updated create-remix-app E2E tests to cover client-side events and transactions.
  • Added create-remix-app-v2 E2E test application that uses 2.0.0 version.

Events:

  • Client-side captured event - Link
  • Client-side render error - Link
  • Server-side error - Link
  • pageload transaction - Link
  • navigation transaction - Link

@onurtemizkan
onurtemizkanforce-pushed the onur/remix-v2-e2e-tests branch 3 times, most recently from 6efcf2c to 0149461CompareSeptember 4, 2023 15:00
@github-actions

github-actionsBot commented Sep 5, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)75.58 KB (0%)
@sentry/browser (incl. Tracing) - Webpack (gzipped)31.49 KB (0%)
@sentry/browser - Webpack (gzipped)22.09 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)70.27 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)28.59 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped)20.66 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)222.15 KB (0%)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)86.64 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)61.49 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)31.47 KB (0%)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)75.61 KB (0%)
@sentry/react - Webpack (gzipped)22.12 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)93.49 KB (0%)
@sentry/nextjs Client - Webpack (gzipped)51.07 KB (0%)

@onurtemizkan
onurtemizkanforce-pushed the onur/remix-v2-e2e-tests branch 2 times, most recently from abb124e to b6ec1a3CompareSeptember 5, 2023 10:31
@onurtemizkanonurtemizkan changed the title feat(remix): Add flags for Remix v2 usage.feat(remix): Add flags for Remix 2.x usage.Sep 5, 2023
@onurtemizkan
onurtemizkanforce-pushed the onur/remix-v2-e2e-tests branch 3 times, most recently from aef6350 to ff4d574CompareSeptember 6, 2023 10:39
@onurtemizkan
onurtemizkan marked this pull request as ready for review September 6, 2023 11:31
AbhiPrasad
AbhiPrasad reviewed Sep 6, 2023
View reviewed changes

@AbhiPrasadAbhiPrasad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we open up a separate PR for the node version changes on the e2e test? That way this PR can only look at changes to remix.

Comment thread.github/workflows/build.yml Outdated
uses: actions/setup-node@v3
with:
node-version-file: 'package.json'
node-version: 18

@AbhiPrasadAbhiPrasadSep 6, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we read the e2e-tests package.json here instead?

@onurtemizkanonurtemizkan mentioned this pull request Sep 7, 2023
Merged
@onurtemizkan
onurtemizkanforce-pushed the onur/remix-v2-e2e-tests branch 3 times, most recently from 9b935d0 to 67c3c4eCompareSeptember 7, 2023 12:46
AbhiPrasad
AbhiPrasad reviewed Sep 8, 2023
View reviewed changes

@AbhiPrasadAbhiPrasad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One last thing before we merge - is there no way to get the remix version automatically? I would prefer not forcing folks to use the isRemixV2 flag.

@onurtemizkan

onurtemizkan commented Sep 8, 2023•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

One last thing before we merge - is there no way to get the remix version automatically? I would prefer not forcing folks to use the isRemixV2 flag.

Yes, I thought about it, there's no property including version available on client side (window.__remixContext), it's possible on server-side though. Is there a reliable way to pass that info on build time from server to client SDK?

Update: Passed remix version from server package.json to browser env while monkey-patching rootloader.

@onurtemizkan
onurtemizkanforce-pushed the onur/remix-v2-e2e-tests branch 4 times, most recently from e62e166 to 09a1425CompareSeptember 11, 2023 18:54
@onurtemizkan
onurtemizkan marked this pull request as draft September 11, 2023 18:55
@onurtemizkan
onurtemizkanforce-pushed the onur/remix-v2-e2e-tests branch 7 times, most recently from 5eb3369 to 2ab1c11CompareSeptember 12, 2023 19:18
@huw

huw commented Sep 16, 2023•
edited
Loading

Copy link
Copy Markdown

Just FYI after upgrading my own setup:

  • error instanceof Error doesn’t work anymore for distinguishing between true client-side and ‘hydrated’ errors; we have to check for the presence of error.stack
  • Remix exports ErrorResponse from all runtimes and isRouteErrorResponse from @remix-run/react works fine for detection in all runtimes
  • ErrorResponses in handleError should probably therefore be sent to captureRemixServerException in the recommended setup (Remix now correctly emits them for Remix-internal errors that don’t end up in the error boundary; we just have to test for status >= 500)
  • captureRemixServerException should check for ErrorResponse instead of Response; I don’t think there are any situations where it would be sent a Response in Remix v2 but am much less sure of this than the others.
  • ErrorResponses that are sent to captureRemixServerException often include errorResponse.error, which is a private field in TypeScript but can be unwrapped to get an ordinary error to send to Sentry. This isn’t true for loader/action ErrorResponses.

Lms24
Lms24 reviewed Sep 18, 2023
View reviewed changes
Comment threadpackages/remix/src/utils/futureFlags.ts Outdated
Comment on lines +50 to +51
const pkg = loadModule<{ version: string }>('@remix-run/react/package.json');
const version = pkg ? pkg.version : '0.0.0';

@Lms24Lms24Sep 18, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

m: If we stick with this approach (see my other comment), let's add a debug log if we fail to load package.json

@sergical

sergical commented Sep 18, 2023

Copy link
Copy Markdown
Member

Remix V2 is out, would love to get this in 😅

@HazATHazAT mentioned this pull request Sep 18, 2023
Closed
@onurtemizkan

onurtemizkan commented Sep 19, 2023

Copy link
Copy Markdown
ContributorAuthor

Thanks for the trial @huw, it's been very helpful!

error instanceof Error doesn’t work anymore for distinguishing between true client-side and ‘hydrated’ errors; we have to check for the presence of error.stack

I was not able to reproduce this, but will take another look after the release.

ErrorResponses that are sent to captureRemixServerException often include errorResponse.error, which is a private field in TypeScript but can be unwrapped to get an ordinary error to send to Sentry. This isn’t true for loader/action ErrorResponses.

This may need a bit of discussion and new test cases.

We can revisit these two points as improvements in other PRs. The other 3 points @huw reported are now included in the first version.

Lms24
Lms24 approved these changes Sep 19, 2023
View reviewed changes

@Lms24Lms24 left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM now! I agree with merging this in now and tackling the open questions in follow up tasks/PRs. Thanks Onur!

Comment threadpackages/e2e-tests/test-applications/create-remix-app-v2/app/entry.client.tsx Outdated
Comment threadpackages/e2e-tests/test-applications/create-remix-app-v2/playwright.config.ts Outdated
@onurtemizkan@Lms24
Apply suggestions from code review
bf7ec96
Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
AbhiPrasad
AbhiPrasad approved these changes Sep 19, 2023
View reviewed changes

@AbhiPrasadAbhiPrasad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's ship this as a first step - thanks Onur!

@AbhiPrasad
AbhiPrasad merged commit 69b308d into developSep 19, 2023
@AbhiPrasad
AbhiPrasad deleted the onur/remix-v2-e2e-tests branch September 19, 2023 13:21
@chohnerchohner mentioned this pull request Sep 19, 2023
Merged
2 tasks
@PrithvirajkumarPrithvirajkumar mentioned this pull request Sep 19, 2023
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Lms24Lms24Lms24 approved these changes
+2 more reviewers
@huwhuwhuw left review comments
@AbhiPrasadAbhiPrasadAbhiPrasad approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Upgrade to remix v2 Make sure Remix SDK works without feature flags

5 participants

@onurtemizkan@huw@sergical@Lms24@AbhiPrasad
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
feat(remix): Add Remix 2.x release support. - #8940
Merged
AbhiPrasad merged 15 commits into
developfrom 
onur/remix-v2-e2e-tests
Sep 19, 2023
Merged
feat(remix): Add Remix 2.x release support.#8940
AbhiPrasad merged 15 commits into
developfrom 
onur/remix-v2-e2e-tests
Conversation
@onurtemizkan
@onurtemizkanonurtemizkan
commented
Sep 4, 2023•

edited by Lms24

Loading

Copy link


Copy Markdown

Contributor
Resolves: #8681
closes#9040
Summary:
  • Updated Node.JS version used on E2E tests to 18.x. (This is required for Remix v2.x, and 18.x is the current LTS so updated this for all) chore(e2e-tests): Use Node 18 for E2E tests. #8964
  • Updated create-remix-app E2E tests to cover client-side events and transactions.
  • Added create-remix-app-v2 E2E test application that uses 2.0.0 version.
Events:
  • Client-side captured event - Link
  • Client-side render error - Link
  • Server-side error - Link
  • pageload transaction - Link
  • navigation transaction - Link
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
6efcf2c to
0149461CompareSeptember 4, 2023 15:00
@github-actions
github-actionsBot
commented
Sep 5, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
size-limit report 📦
PathSize
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)75.58 KB (0%)
@sentry/browser (incl. Tracing) - Webpack (gzipped)31.49 KB (0%)
@sentry/browser - Webpack (gzipped)22.09 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)70.27 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)28.59 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped)20.66 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)222.15 KB (0%)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)86.64 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)61.49 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)31.47 KB (0%)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)75.61 KB (0%)
@sentry/react - Webpack (gzipped)22.12 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)93.49 KB (0%)
@sentry/nextjs Client - Webpack (gzipped)51.07 KB (0%)
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
2 times, most recently
from
abb124e to
b6ec1a3CompareSeptember 5, 2023 10:31
@onurtemizkanonurtemizkan
changed the title
feat(remix): Add flags for Remix v2 usage.feat(remix): Add flags for Remix 2.x usage.Sep 5, 2023
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
aef6350 to
ff4d574CompareSeptember 6, 2023 10:39
@onurtemizkan
onurtemizkan
marked this pull request as ready for review
September 6, 2023 11:31
AbhiPrasad
AbhiPrasad
reviewed
Sep 6, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Can we open up a separate PR for the node version changes on the e2e test? That way this PR can only look at changes to remix.
Comment thread.github/workflows/build.yml
Outdated

 uses: actions/setup-node@v3
 with:
 node-version-file: 'package.json'
 node-version: 18
@AbhiPrasadAbhiPrasadSep 6, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Can we read the e2e-tests package.json here instead?
@onurtemizkanonurtemizkan
mentioned this pull request
Sep 7, 2023
 Merged

@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
9b935d0 to
67c3c4eCompareSeptember 7, 2023 12:46
AbhiPrasad
AbhiPrasad
reviewed
Sep 8, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

One last thing before we merge - is there no way to get the remix version automatically? I would prefer not forcing folks to use the isRemixV2 flag.
@onurtemizkan
onurtemizkan
commented
Sep 8, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
One last thing before we merge - is there no way to get the remix version automatically? I would prefer not forcing folks to use the isRemixV2 flag.
Yes, I thought about it, there's no property including version available on client side (window.__remixContext), it's possible on server-side though. Is there a reliable way to pass that info on build time from server to client SDK?
Update: Passed remix version from server package.json to browser env while monkey-patching rootloader.
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
4 times, most recently
from
e62e166 to
09a1425CompareSeptember 11, 2023 18:54
@onurtemizkan
onurtemizkan
marked this pull request as draft
September 11, 2023 18:55
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
7 times, most recently
from
5eb3369 to
2ab1c11CompareSeptember 12, 2023 19:18
@huw
huw
commented
Sep 16, 2023•

edited

Loading

Copy link


Copy Markdown

Just FYI after upgrading my own setup:
  • error instanceof Error doesn’t work anymore for distinguishing between true client-side and ‘hydrated’ errors; we have to check for the presence of error.stack
  • Remix exports ErrorResponse from all runtimes and isRouteErrorResponse from @remix-run/react works fine for detection in all runtimes
  • ErrorResponses in handleError should probably therefore be sent to captureRemixServerException in the recommended setup (Remix now correctly emits them for Remix-internal errors that don’t end up in the error boundary; we just have to test for status >= 500)
  • captureRemixServerException should check for ErrorResponse instead of Response; I don’t think there are any situations where it would be sent a Response in Remix v2 but am much less sure of this than the others.
  • ErrorResponses that are sent to captureRemixServerException often include errorResponse.error, which is a private field in TypeScript but can be unwrapped to get an ordinary error to send to Sentry. This isn’t true for loader/action ErrorResponses.
Lms24
Lms24
reviewed
Sep 18, 2023
View reviewed changes
Comment threadpackages/remix/src/utils/futureFlags.ts
Outdated


Comment on lines

+50
 to 
+51

 const pkg = loadModule<{ version: string }>('@remix-run/react/package.json');
 const version = pkg ? pkg.version : '0.0.0';
@Lms24Lms24Sep 18, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

m: If we stick with this approach (see my other comment), let's add a debug log if we fail to load package.json
@sergical
sergical
commented
Sep 18, 2023

Copy link


Copy Markdown

Member
Remix V2 is out, would love to get this in 😅
@HazATHazAT
mentioned this pull request
Sep 18, 2023

Closed

@onurtemizkan
onurtemizkan
commented
Sep 19, 2023

Copy link


Copy Markdown

ContributorAuthor
Thanks for the trial @huw, it's been very helpful!
error instanceof Error doesn’t work anymore for distinguishing between true client-side and ‘hydrated’ errors; we have to check for the presence of error.stack
I was not able to reproduce this, but will take another look after the release.
ErrorResponses that are sent to captureRemixServerException often include errorResponse.error, which is a private field in TypeScript but can be unwrapped to get an ordinary error to send to Sentry. This isn’t true for loader/action ErrorResponses.
This may need a bit of discussion and new test cases.
We can revisit these two points as improvements in other PRs. The other 3 points @huw reported are now included in the first version.
Lms24
Lms24
approved these changes
Sep 19, 2023
View reviewed changes
@Lms24Lms24
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM now! I agree with merging this in now and tackling the open questions in follow up tasks/PRs. Thanks Onur!
Comment threadpackages/e2e-tests/test-applications/create-remix-app-v2/app/entry.client.tsx
Outdated

Comment threadpackages/e2e-tests/test-applications/create-remix-app-v2/playwright.config.ts
Outdated

@onurtemizkan@Lms24
Apply suggestions from code review
bf7ec96
Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
AbhiPrasad
AbhiPrasad
approved these changes
Sep 19, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Let's ship this as a first step - thanks Onur!
@AbhiPrasad
AbhiPrasad
merged commit 69b308d
into
developSep 19, 2023
@AbhiPrasad
AbhiPrasad
deleted the

onur/remix-v2-e2e-tests

branch
September 19, 2023 13:21
@chohnerchohner
mentioned this pull request
Sep 19, 2023
 Merged

2 tasks
@PrithvirajkumarPrithvirajkumar
mentioned this pull request
Sep 19, 2023

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@Lms24Lms24Lms24 approved these changes

+2 more reviewers

@huwhuwhuw left review comments
@AbhiPrasadAbhiPrasadAbhiPrasad approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Upgrade to remix v2

Make sure Remix SDK works without feature flags


5 participants

@onurtemizkan@huw@sergical@Lms24@AbhiPrasad

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
feat(remix): Add Remix 2.x release support. - #8940
Merged
AbhiPrasad merged 15 commits into
developfrom 
onur/remix-v2-e2e-tests
Sep 19, 2023
Merged
feat(remix): Add Remix 2.x release support.#8940
AbhiPrasad merged 15 commits into
developfrom 
onur/remix-v2-e2e-tests
Conversation
@onurtemizkan
@onurtemizkanonurtemizkan
commented
Sep 4, 2023•

edited by Lms24

Loading

Copy link


Copy Markdown

Contributor
Resolves: #8681
closes#9040
Summary:
  • Updated Node.JS version used on E2E tests to 18.x. (This is required for Remix v2.x, and 18.x is the current LTS so updated this for all) chore(e2e-tests): Use Node 18 for E2E tests. #8964
  • Updated create-remix-app E2E tests to cover client-side events and transactions.
  • Added create-remix-app-v2 E2E test application that uses 2.0.0 version.
Events:
  • Client-side captured event - Link
  • Client-side render error - Link
  • Server-side error - Link
  • pageload transaction - Link
  • navigation transaction - Link
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
6efcf2c to
0149461CompareSeptember 4, 2023 15:00
@github-actions
github-actionsBot
commented
Sep 5, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
size-limit report 📦
PathSize
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)75.58 KB (0%)
@sentry/browser (incl. Tracing) - Webpack (gzipped)31.49 KB (0%)
@sentry/browser - Webpack (gzipped)22.09 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)70.27 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)28.59 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped)20.66 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)222.15 KB (0%)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)86.64 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)61.49 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)31.47 KB (0%)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)75.61 KB (0%)
@sentry/react - Webpack (gzipped)22.12 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)93.49 KB (0%)
@sentry/nextjs Client - Webpack (gzipped)51.07 KB (0%)
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
2 times, most recently
from
abb124e to
b6ec1a3CompareSeptember 5, 2023 10:31
@onurtemizkanonurtemizkan
changed the title
feat(remix): Add flags for Remix v2 usage.feat(remix): Add flags for Remix 2.x usage.Sep 5, 2023
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
aef6350 to
ff4d574CompareSeptember 6, 2023 10:39
@onurtemizkan
onurtemizkan
marked this pull request as ready for review
September 6, 2023 11:31
AbhiPrasad
AbhiPrasad
reviewed
Sep 6, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Can we open up a separate PR for the node version changes on the e2e test? That way this PR can only look at changes to remix.
Comment thread.github/workflows/build.yml
Outdated

 uses: actions/setup-node@v3
 with:
 node-version-file: 'package.json'
 node-version: 18
@AbhiPrasadAbhiPrasadSep 6, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Can we read the e2e-tests package.json here instead?
@onurtemizkanonurtemizkan
mentioned this pull request
Sep 7, 2023
 Merged

@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
9b935d0 to
67c3c4eCompareSeptember 7, 2023 12:46
AbhiPrasad
AbhiPrasad
reviewed
Sep 8, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

One last thing before we merge - is there no way to get the remix version automatically? I would prefer not forcing folks to use the isRemixV2 flag.
@onurtemizkan
onurtemizkan
commented
Sep 8, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
One last thing before we merge - is there no way to get the remix version automatically? I would prefer not forcing folks to use the isRemixV2 flag.
Yes, I thought about it, there's no property including version available on client side (window.__remixContext), it's possible on server-side though. Is there a reliable way to pass that info on build time from server to client SDK?
Update: Passed remix version from server package.json to browser env while monkey-patching rootloader.
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
4 times, most recently
from
e62e166 to
09a1425CompareSeptember 11, 2023 18:54
@onurtemizkan
onurtemizkan
marked this pull request as draft
September 11, 2023 18:55
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
7 times, most recently
from
5eb3369 to
2ab1c11CompareSeptember 12, 2023 19:18
@huw
huw
commented
Sep 16, 2023•

edited

Loading

Copy link


Copy Markdown

Just FYI after upgrading my own setup:
  • error instanceof Error doesn’t work anymore for distinguishing between true client-side and ‘hydrated’ errors; we have to check for the presence of error.stack
  • Remix exports ErrorResponse from all runtimes and isRouteErrorResponse from @remix-run/react works fine for detection in all runtimes
  • ErrorResponses in handleError should probably therefore be sent to captureRemixServerException in the recommended setup (Remix now correctly emits them for Remix-internal errors that don’t end up in the error boundary; we just have to test for status >= 500)
  • captureRemixServerException should check for ErrorResponse instead of Response; I don’t think there are any situations where it would be sent a Response in Remix v2 but am much less sure of this than the others.
  • ErrorResponses that are sent to captureRemixServerException often include errorResponse.error, which is a private field in TypeScript but can be unwrapped to get an ordinary error to send to Sentry. This isn’t true for loader/action ErrorResponses.
Lms24
Lms24
reviewed
Sep 18, 2023
View reviewed changes
Comment threadpackages/remix/src/utils/futureFlags.ts
Outdated


Comment on lines

+50
 to 
+51

 const pkg = loadModule<{ version: string }>('@remix-run/react/package.json');
 const version = pkg ? pkg.version : '0.0.0';
@Lms24Lms24Sep 18, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

m: If we stick with this approach (see my other comment), let's add a debug log if we fail to load package.json
@sergical
sergical
commented
Sep 18, 2023

Copy link


Copy Markdown

Member
Remix V2 is out, would love to get this in 😅
@HazATHazAT
mentioned this pull request
Sep 18, 2023

Closed

@onurtemizkan
onurtemizkan
commented
Sep 19, 2023

Copy link


Copy Markdown

ContributorAuthor
Thanks for the trial @huw, it's been very helpful!
error instanceof Error doesn’t work anymore for distinguishing between true client-side and ‘hydrated’ errors; we have to check for the presence of error.stack
I was not able to reproduce this, but will take another look after the release.
ErrorResponses that are sent to captureRemixServerException often include errorResponse.error, which is a private field in TypeScript but can be unwrapped to get an ordinary error to send to Sentry. This isn’t true for loader/action ErrorResponses.
This may need a bit of discussion and new test cases.
We can revisit these two points as improvements in other PRs. The other 3 points @huw reported are now included in the first version.
Lms24
Lms24
approved these changes
Sep 19, 2023
View reviewed changes
@Lms24Lms24
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM now! I agree with merging this in now and tackling the open questions in follow up tasks/PRs. Thanks Onur!
Comment threadpackages/e2e-tests/test-applications/create-remix-app-v2/app/entry.client.tsx
Outdated

Comment threadpackages/e2e-tests/test-applications/create-remix-app-v2/playwright.config.ts
Outdated

@onurtemizkan@Lms24
Apply suggestions from code review
bf7ec96
Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
AbhiPrasad
AbhiPrasad
approved these changes
Sep 19, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Let's ship this as a first step - thanks Onur!
@AbhiPrasad
AbhiPrasad
merged commit 69b308d
into
developSep 19, 2023
@AbhiPrasad
AbhiPrasad
deleted the

onur/remix-v2-e2e-tests

branch
September 19, 2023 13:21
@chohnerchohner
mentioned this pull request
Sep 19, 2023
 Merged

2 tasks
@PrithvirajkumarPrithvirajkumar
mentioned this pull request
Sep 19, 2023

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@Lms24Lms24Lms24 approved these changes

+2 more reviewers

@huwhuwhuw left review comments
@AbhiPrasadAbhiPrasadAbhiPrasad approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Upgrade to remix v2

Make sure Remix SDK works without feature flags


5 participants

@onurtemizkan@huw@sergical@Lms24@AbhiPrasad

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
feat(remix): Add Remix 2.x release support. - #8940
Merged
AbhiPrasad merged 15 commits into
developfrom 
onur/remix-v2-e2e-tests
Sep 19, 2023
Merged
feat(remix): Add Remix 2.x release support.#8940
AbhiPrasad merged 15 commits into
developfrom 
onur/remix-v2-e2e-tests
Conversation
@onurtemizkan
@onurtemizkanonurtemizkan
commented
Sep 4, 2023•

edited by Lms24

Loading

Copy link


Copy Markdown

Contributor
Resolves: #8681
closes#9040
Summary:
  • Updated Node.JS version used on E2E tests to 18.x. (This is required for Remix v2.x, and 18.x is the current LTS so updated this for all) chore(e2e-tests): Use Node 18 for E2E tests. #8964
  • Updated create-remix-app E2E tests to cover client-side events and transactions.
  • Added create-remix-app-v2 E2E test application that uses 2.0.0 version.
Events:
  • Client-side captured event - Link
  • Client-side render error - Link
  • Server-side error - Link
  • pageload transaction - Link
  • navigation transaction - Link
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
6efcf2c to
0149461CompareSeptember 4, 2023 15:00
@github-actions
github-actionsBot
commented
Sep 5, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
size-limit report 📦
PathSize
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)75.58 KB (0%)
@sentry/browser (incl. Tracing) - Webpack (gzipped)31.49 KB (0%)
@sentry/browser - Webpack (gzipped)22.09 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)70.27 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)28.59 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped)20.66 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)222.15 KB (0%)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)86.64 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)61.49 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)31.47 KB (0%)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)75.61 KB (0%)
@sentry/react - Webpack (gzipped)22.12 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)93.49 KB (0%)
@sentry/nextjs Client - Webpack (gzipped)51.07 KB (0%)
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
2 times, most recently
from
abb124e to
b6ec1a3CompareSeptember 5, 2023 10:31
@onurtemizkanonurtemizkan
changed the title
feat(remix): Add flags for Remix v2 usage.feat(remix): Add flags for Remix 2.x usage.Sep 5, 2023
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
aef6350 to
ff4d574CompareSeptember 6, 2023 10:39
@onurtemizkan
onurtemizkan
marked this pull request as ready for review
September 6, 2023 11:31
AbhiPrasad
AbhiPrasad
reviewed
Sep 6, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Can we open up a separate PR for the node version changes on the e2e test? That way this PR can only look at changes to remix.
Comment thread.github/workflows/build.yml
Outdated

 uses: actions/setup-node@v3
 with:
 node-version-file: 'package.json'
 node-version: 18
@AbhiPrasadAbhiPrasadSep 6, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Can we read the e2e-tests package.json here instead?
@onurtemizkanonurtemizkan
mentioned this pull request
Sep 7, 2023
 Merged

@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
9b935d0 to
67c3c4eCompareSeptember 7, 2023 12:46
AbhiPrasad
AbhiPrasad
reviewed
Sep 8, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

One last thing before we merge - is there no way to get the remix version automatically? I would prefer not forcing folks to use the isRemixV2 flag.
@onurtemizkan
onurtemizkan
commented
Sep 8, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
One last thing before we merge - is there no way to get the remix version automatically? I would prefer not forcing folks to use the isRemixV2 flag.
Yes, I thought about it, there's no property including version available on client side (window.__remixContext), it's possible on server-side though. Is there a reliable way to pass that info on build time from server to client SDK?
Update: Passed remix version from server package.json to browser env while monkey-patching rootloader.
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
4 times, most recently
from
e62e166 to
09a1425CompareSeptember 11, 2023 18:54
@onurtemizkan
onurtemizkan
marked this pull request as draft
September 11, 2023 18:55
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
7 times, most recently
from
5eb3369 to
2ab1c11CompareSeptember 12, 2023 19:18
@huw
huw
commented
Sep 16, 2023•

edited

Loading

Copy link


Copy Markdown

Just FYI after upgrading my own setup:
  • error instanceof Error doesn’t work anymore for distinguishing between true client-side and ‘hydrated’ errors; we have to check for the presence of error.stack
  • Remix exports ErrorResponse from all runtimes and isRouteErrorResponse from @remix-run/react works fine for detection in all runtimes
  • ErrorResponses in handleError should probably therefore be sent to captureRemixServerException in the recommended setup (Remix now correctly emits them for Remix-internal errors that don’t end up in the error boundary; we just have to test for status >= 500)
  • captureRemixServerException should check for ErrorResponse instead of Response; I don’t think there are any situations where it would be sent a Response in Remix v2 but am much less sure of this than the others.
  • ErrorResponses that are sent to captureRemixServerException often include errorResponse.error, which is a private field in TypeScript but can be unwrapped to get an ordinary error to send to Sentry. This isn’t true for loader/action ErrorResponses.
Lms24
Lms24
reviewed
Sep 18, 2023
View reviewed changes
Comment threadpackages/remix/src/utils/futureFlags.ts
Outdated


Comment on lines

+50
 to 
+51

 const pkg = loadModule<{ version: string }>('@remix-run/react/package.json');
 const version = pkg ? pkg.version : '0.0.0';
@Lms24Lms24Sep 18, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

m: If we stick with this approach (see my other comment), let's add a debug log if we fail to load package.json
@sergical
sergical
commented
Sep 18, 2023

Copy link


Copy Markdown

Member
Remix V2 is out, would love to get this in 😅
@HazATHazAT
mentioned this pull request
Sep 18, 2023

Closed

@onurtemizkan
onurtemizkan
commented
Sep 19, 2023

Copy link


Copy Markdown

ContributorAuthor
Thanks for the trial @huw, it's been very helpful!
error instanceof Error doesn’t work anymore for distinguishing between true client-side and ‘hydrated’ errors; we have to check for the presence of error.stack
I was not able to reproduce this, but will take another look after the release.
ErrorResponses that are sent to captureRemixServerException often include errorResponse.error, which is a private field in TypeScript but can be unwrapped to get an ordinary error to send to Sentry. This isn’t true for loader/action ErrorResponses.
This may need a bit of discussion and new test cases.
We can revisit these two points as improvements in other PRs. The other 3 points @huw reported are now included in the first version.
Lms24
Lms24
approved these changes
Sep 19, 2023
View reviewed changes
@Lms24Lms24
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM now! I agree with merging this in now and tackling the open questions in follow up tasks/PRs. Thanks Onur!
Comment threadpackages/e2e-tests/test-applications/create-remix-app-v2/app/entry.client.tsx
Outdated

Comment threadpackages/e2e-tests/test-applications/create-remix-app-v2/playwright.config.ts
Outdated

@onurtemizkan@Lms24
Apply suggestions from code review
bf7ec96
Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
AbhiPrasad
AbhiPrasad
approved these changes
Sep 19, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Let's ship this as a first step - thanks Onur!
@AbhiPrasad
AbhiPrasad
merged commit 69b308d
into
developSep 19, 2023
@AbhiPrasad
AbhiPrasad
deleted the

onur/remix-v2-e2e-tests

branch
September 19, 2023 13:21
@chohnerchohner
mentioned this pull request
Sep 19, 2023
 Merged

2 tasks
@PrithvirajkumarPrithvirajkumar
mentioned this pull request
Sep 19, 2023

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@Lms24Lms24Lms24 approved these changes

+2 more reviewers

@huwhuwhuw left review comments
@AbhiPrasadAbhiPrasadAbhiPrasad approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Upgrade to remix v2

Make sure Remix SDK works without feature flags


5 participants

@onurtemizkan@huw@sergical@Lms24@AbhiPrasad

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
feat(remix): Add Remix 2.x release support. - #8940
Merged
AbhiPrasad merged 15 commits into
developfrom 
onur/remix-v2-e2e-tests
Sep 19, 2023
Merged
feat(remix): Add Remix 2.x release support.#8940
AbhiPrasad merged 15 commits into
developfrom 
onur/remix-v2-e2e-tests
Conversation
@onurtemizkan
@onurtemizkanonurtemizkan
commented
Sep 4, 2023•

edited by Lms24

Loading

Copy link


Copy Markdown

Contributor
Resolves: #8681
closes#9040
Summary:
  • Updated Node.JS version used on E2E tests to 18.x. (This is required for Remix v2.x, and 18.x is the current LTS so updated this for all) chore(e2e-tests): Use Node 18 for E2E tests. #8964
  • Updated create-remix-app E2E tests to cover client-side events and transactions.
  • Added create-remix-app-v2 E2E test application that uses 2.0.0 version.
Events:
  • Client-side captured event - Link
  • Client-side render error - Link
  • Server-side error - Link
  • pageload transaction - Link
  • navigation transaction - Link
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
6efcf2c to
0149461CompareSeptember 4, 2023 15:00
@github-actions
github-actionsBot
commented
Sep 5, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
size-limit report 📦
PathSize
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)75.58 KB (0%)
@sentry/browser (incl. Tracing) - Webpack (gzipped)31.49 KB (0%)
@sentry/browser - Webpack (gzipped)22.09 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)70.27 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)28.59 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped)20.66 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)222.15 KB (0%)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)86.64 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)61.49 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)31.47 KB (0%)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)75.61 KB (0%)
@sentry/react - Webpack (gzipped)22.12 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)93.49 KB (0%)
@sentry/nextjs Client - Webpack (gzipped)51.07 KB (0%)
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
2 times, most recently
from
abb124e to
b6ec1a3CompareSeptember 5, 2023 10:31
@onurtemizkanonurtemizkan
changed the title
feat(remix): Add flags for Remix v2 usage.feat(remix): Add flags for Remix 2.x usage.Sep 5, 2023
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
aef6350 to
ff4d574CompareSeptember 6, 2023 10:39
@onurtemizkan
onurtemizkan
marked this pull request as ready for review
September 6, 2023 11:31
AbhiPrasad
AbhiPrasad
reviewed
Sep 6, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Can we open up a separate PR for the node version changes on the e2e test? That way this PR can only look at changes to remix.
Comment thread.github/workflows/build.yml
Outdated

 uses: actions/setup-node@v3
 with:
 node-version-file: 'package.json'
 node-version: 18
@AbhiPrasadAbhiPrasadSep 6, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Can we read the e2e-tests package.json here instead?
@onurtemizkanonurtemizkan
mentioned this pull request
Sep 7, 2023
 Merged

@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
9b935d0 to
67c3c4eCompareSeptember 7, 2023 12:46
AbhiPrasad
AbhiPrasad
reviewed
Sep 8, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

One last thing before we merge - is there no way to get the remix version automatically? I would prefer not forcing folks to use the isRemixV2 flag.
@onurtemizkan
onurtemizkan
commented
Sep 8, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
One last thing before we merge - is there no way to get the remix version automatically? I would prefer not forcing folks to use the isRemixV2 flag.
Yes, I thought about it, there's no property including version available on client side (window.__remixContext), it's possible on server-side though. Is there a reliable way to pass that info on build time from server to client SDK?
Update: Passed remix version from server package.json to browser env while monkey-patching rootloader.
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
4 times, most recently
from
e62e166 to
09a1425CompareSeptember 11, 2023 18:54
@onurtemizkan
onurtemizkan
marked this pull request as draft
September 11, 2023 18:55
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
7 times, most recently
from
5eb3369 to
2ab1c11CompareSeptember 12, 2023 19:18
@huw
huw
commented
Sep 16, 2023•

edited

Loading

Copy link


Copy Markdown

Just FYI after upgrading my own setup:
  • error instanceof Error doesn’t work anymore for distinguishing between true client-side and ‘hydrated’ errors; we have to check for the presence of error.stack
  • Remix exports ErrorResponse from all runtimes and isRouteErrorResponse from @remix-run/react works fine for detection in all runtimes
  • ErrorResponses in handleError should probably therefore be sent to captureRemixServerException in the recommended setup (Remix now correctly emits them for Remix-internal errors that don’t end up in the error boundary; we just have to test for status >= 500)
  • captureRemixServerException should check for ErrorResponse instead of Response; I don’t think there are any situations where it would be sent a Response in Remix v2 but am much less sure of this than the others.
  • ErrorResponses that are sent to captureRemixServerException often include errorResponse.error, which is a private field in TypeScript but can be unwrapped to get an ordinary error to send to Sentry. This isn’t true for loader/action ErrorResponses.
Lms24
Lms24
reviewed
Sep 18, 2023
View reviewed changes
Comment threadpackages/remix/src/utils/futureFlags.ts
Outdated


Comment on lines

+50
 to 
+51

 const pkg = loadModule<{ version: string }>('@remix-run/react/package.json');
 const version = pkg ? pkg.version : '0.0.0';
@Lms24Lms24Sep 18, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

m: If we stick with this approach (see my other comment), let's add a debug log if we fail to load package.json
@sergical
sergical
commented
Sep 18, 2023

Copy link


Copy Markdown

Member
Remix V2 is out, would love to get this in 😅
@HazATHazAT
mentioned this pull request
Sep 18, 2023

Closed

@onurtemizkan
onurtemizkan
commented
Sep 19, 2023

Copy link


Copy Markdown

ContributorAuthor
Thanks for the trial @huw, it's been very helpful!
error instanceof Error doesn’t work anymore for distinguishing between true client-side and ‘hydrated’ errors; we have to check for the presence of error.stack
I was not able to reproduce this, but will take another look after the release.
ErrorResponses that are sent to captureRemixServerException often include errorResponse.error, which is a private field in TypeScript but can be unwrapped to get an ordinary error to send to Sentry. This isn’t true for loader/action ErrorResponses.
This may need a bit of discussion and new test cases.
We can revisit these two points as improvements in other PRs. The other 3 points @huw reported are now included in the first version.
Lms24
Lms24
approved these changes
Sep 19, 2023
View reviewed changes
@Lms24Lms24
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM now! I agree with merging this in now and tackling the open questions in follow up tasks/PRs. Thanks Onur!
Comment threadpackages/e2e-tests/test-applications/create-remix-app-v2/app/entry.client.tsx
Outdated

Comment threadpackages/e2e-tests/test-applications/create-remix-app-v2/playwright.config.ts
Outdated

@onurtemizkan@Lms24
Apply suggestions from code review
bf7ec96
Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
AbhiPrasad
AbhiPrasad
approved these changes
Sep 19, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Let's ship this as a first step - thanks Onur!
@AbhiPrasad
AbhiPrasad
merged commit 69b308d
into
developSep 19, 2023
@AbhiPrasad
AbhiPrasad
deleted the

onur/remix-v2-e2e-tests

branch
September 19, 2023 13:21
@chohnerchohner
mentioned this pull request
Sep 19, 2023
 Merged

2 tasks
@PrithvirajkumarPrithvirajkumar
mentioned this pull request
Sep 19, 2023

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@Lms24Lms24Lms24 approved these changes

+2 more reviewers

@huwhuwhuw left review comments
@AbhiPrasadAbhiPrasadAbhiPrasad approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Upgrade to remix v2

Make sure Remix SDK works without feature flags


5 participants

@onurtemizkan@huw@sergical@Lms24@AbhiPrasad

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
feat(remix): Add Remix 2.x release support. - #8940
Merged
AbhiPrasad merged 15 commits into
developfrom 
onur/remix-v2-e2e-tests
Sep 19, 2023
Merged
feat(remix): Add Remix 2.x release support.#8940
AbhiPrasad merged 15 commits into
developfrom 
onur/remix-v2-e2e-tests
Conversation
@onurtemizkan
@onurtemizkanonurtemizkan
commented
Sep 4, 2023•

edited by Lms24

Loading

Copy link


Copy Markdown

Contributor
Resolves: #8681
closes#9040
Summary:
  • Updated Node.JS version used on E2E tests to 18.x. (This is required for Remix v2.x, and 18.x is the current LTS so updated this for all) chore(e2e-tests): Use Node 18 for E2E tests. #8964
  • Updated create-remix-app E2E tests to cover client-side events and transactions.
  • Added create-remix-app-v2 E2E test application that uses 2.0.0 version.
Events:
  • Client-side captured event - Link
  • Client-side render error - Link
  • Server-side error - Link
  • pageload transaction - Link
  • navigation transaction - Link
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
6efcf2c to
0149461CompareSeptember 4, 2023 15:00
@github-actions
github-actionsBot
commented
Sep 5, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
size-limit report 📦
PathSize
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)75.58 KB (0%)
@sentry/browser (incl. Tracing) - Webpack (gzipped)31.49 KB (0%)
@sentry/browser - Webpack (gzipped)22.09 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)70.27 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)28.59 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped)20.66 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)222.15 KB (0%)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)86.64 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)61.49 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)31.47 KB (0%)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)75.61 KB (0%)
@sentry/react - Webpack (gzipped)22.12 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)93.49 KB (0%)
@sentry/nextjs Client - Webpack (gzipped)51.07 KB (0%)
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
2 times, most recently
from
abb124e to
b6ec1a3CompareSeptember 5, 2023 10:31
@onurtemizkanonurtemizkan
changed the title
feat(remix): Add flags for Remix v2 usage.feat(remix): Add flags for Remix 2.x usage.Sep 5, 2023
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
aef6350 to
ff4d574CompareSeptember 6, 2023 10:39
@onurtemizkan
onurtemizkan
marked this pull request as ready for review
September 6, 2023 11:31
AbhiPrasad
AbhiPrasad
reviewed
Sep 6, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Can we open up a separate PR for the node version changes on the e2e test? That way this PR can only look at changes to remix.
Comment thread.github/workflows/build.yml
Outdated

 uses: actions/setup-node@v3
 with:
 node-version-file: 'package.json'
 node-version: 18
@AbhiPrasadAbhiPrasadSep 6, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Can we read the e2e-tests package.json here instead?
@onurtemizkanonurtemizkan
mentioned this pull request
Sep 7, 2023
 Merged

@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
9b935d0 to
67c3c4eCompareSeptember 7, 2023 12:46
AbhiPrasad
AbhiPrasad
reviewed
Sep 8, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

One last thing before we merge - is there no way to get the remix version automatically? I would prefer not forcing folks to use the isRemixV2 flag.
@onurtemizkan
onurtemizkan
commented
Sep 8, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
One last thing before we merge - is there no way to get the remix version automatically? I would prefer not forcing folks to use the isRemixV2 flag.
Yes, I thought about it, there's no property including version available on client side (window.__remixContext), it's possible on server-side though. Is there a reliable way to pass that info on build time from server to client SDK?
Update: Passed remix version from server package.json to browser env while monkey-patching rootloader.
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
4 times, most recently
from
e62e166 to
09a1425CompareSeptember 11, 2023 18:54
@onurtemizkan
onurtemizkan
marked this pull request as draft
September 11, 2023 18:55
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
7 times, most recently
from
5eb3369 to
2ab1c11CompareSeptember 12, 2023 19:18
@huw
huw
commented
Sep 16, 2023•

edited

Loading

Copy link


Copy Markdown

Just FYI after upgrading my own setup:
  • error instanceof Error doesn’t work anymore for distinguishing between true client-side and ‘hydrated’ errors; we have to check for the presence of error.stack
  • Remix exports ErrorResponse from all runtimes and isRouteErrorResponse from @remix-run/react works fine for detection in all runtimes
  • ErrorResponses in handleError should probably therefore be sent to captureRemixServerException in the recommended setup (Remix now correctly emits them for Remix-internal errors that don’t end up in the error boundary; we just have to test for status >= 500)
  • captureRemixServerException should check for ErrorResponse instead of Response; I don’t think there are any situations where it would be sent a Response in Remix v2 but am much less sure of this than the others.
  • ErrorResponses that are sent to captureRemixServerException often include errorResponse.error, which is a private field in TypeScript but can be unwrapped to get an ordinary error to send to Sentry. This isn’t true for loader/action ErrorResponses.
Lms24
Lms24
reviewed
Sep 18, 2023
View reviewed changes
Comment threadpackages/remix/src/utils/futureFlags.ts
Outdated


Comment on lines

+50
 to 
+51

 const pkg = loadModule<{ version: string }>('@remix-run/react/package.json');
 const version = pkg ? pkg.version : '0.0.0';
@Lms24Lms24Sep 18, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

m: If we stick with this approach (see my other comment), let's add a debug log if we fail to load package.json
@sergical
sergical
commented
Sep 18, 2023

Copy link


Copy Markdown

Member
Remix V2 is out, would love to get this in 😅
@HazATHazAT
mentioned this pull request
Sep 18, 2023

Closed

@onurtemizkan
onurtemizkan
commented
Sep 19, 2023

Copy link


Copy Markdown

ContributorAuthor
Thanks for the trial @huw, it's been very helpful!
error instanceof Error doesn’t work anymore for distinguishing between true client-side and ‘hydrated’ errors; we have to check for the presence of error.stack
I was not able to reproduce this, but will take another look after the release.
ErrorResponses that are sent to captureRemixServerException often include errorResponse.error, which is a private field in TypeScript but can be unwrapped to get an ordinary error to send to Sentry. This isn’t true for loader/action ErrorResponses.
This may need a bit of discussion and new test cases.
We can revisit these two points as improvements in other PRs. The other 3 points @huw reported are now included in the first version.
Lms24
Lms24
approved these changes
Sep 19, 2023
View reviewed changes
@Lms24Lms24
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM now! I agree with merging this in now and tackling the open questions in follow up tasks/PRs. Thanks Onur!
Comment threadpackages/e2e-tests/test-applications/create-remix-app-v2/app/entry.client.tsx
Outdated

Comment threadpackages/e2e-tests/test-applications/create-remix-app-v2/playwright.config.ts
Outdated

@onurtemizkan@Lms24
Apply suggestions from code review
bf7ec96
Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
AbhiPrasad
AbhiPrasad
approved these changes
Sep 19, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Let's ship this as a first step - thanks Onur!
@AbhiPrasad
AbhiPrasad
merged commit 69b308d
into
developSep 19, 2023
@AbhiPrasad
AbhiPrasad
deleted the

onur/remix-v2-e2e-tests

branch
September 19, 2023 13:21
@chohnerchohner
mentioned this pull request
Sep 19, 2023
 Merged

2 tasks
@PrithvirajkumarPrithvirajkumar
mentioned this pull request
Sep 19, 2023

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@Lms24Lms24Lms24 approved these changes

+2 more reviewers

@huwhuwhuw left review comments
@AbhiPrasadAbhiPrasadAbhiPrasad approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Upgrade to remix v2

Make sure Remix SDK works without feature flags


5 participants

@onurtemizkan@huw@sergical@Lms24@AbhiPrasad

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
feat(remix): Add Remix 2.x release support. - #8940
Merged
AbhiPrasad merged 15 commits into
developfrom 
onur/remix-v2-e2e-tests
Sep 19, 2023
Merged
feat(remix): Add Remix 2.x release support.#8940
AbhiPrasad merged 15 commits into
developfrom 
onur/remix-v2-e2e-tests
Conversation
@onurtemizkan
@onurtemizkanonurtemizkan
commented
Sep 4, 2023•

edited by Lms24

Loading

Copy link


Copy Markdown

Contributor
Resolves: #8681
closes#9040
Summary:
  • Updated Node.JS version used on E2E tests to 18.x. (This is required for Remix v2.x, and 18.x is the current LTS so updated this for all) chore(e2e-tests): Use Node 18 for E2E tests. #8964
  • Updated create-remix-app E2E tests to cover client-side events and transactions.
  • Added create-remix-app-v2 E2E test application that uses 2.0.0 version.
Events:
  • Client-side captured event - Link
  • Client-side render error - Link
  • Server-side error - Link
  • pageload transaction - Link
  • navigation transaction - Link
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
6efcf2c to
0149461CompareSeptember 4, 2023 15:00
@github-actions
github-actionsBot
commented
Sep 5, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
size-limit report 📦
PathSize
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)75.58 KB (0%)
@sentry/browser (incl. Tracing) - Webpack (gzipped)31.49 KB (0%)
@sentry/browser - Webpack (gzipped)22.09 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)70.27 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)28.59 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped)20.66 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)222.15 KB (0%)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)86.64 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)61.49 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)31.47 KB (0%)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)75.61 KB (0%)
@sentry/react - Webpack (gzipped)22.12 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)93.49 KB (0%)
@sentry/nextjs Client - Webpack (gzipped)51.07 KB (0%)
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
2 times, most recently
from
abb124e to
b6ec1a3CompareSeptember 5, 2023 10:31
@onurtemizkanonurtemizkan
changed the title
feat(remix): Add flags for Remix v2 usage.feat(remix): Add flags for Remix 2.x usage.Sep 5, 2023
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
aef6350 to
ff4d574CompareSeptember 6, 2023 10:39
@onurtemizkan
onurtemizkan
marked this pull request as ready for review
September 6, 2023 11:31
AbhiPrasad
AbhiPrasad
reviewed
Sep 6, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Can we open up a separate PR for the node version changes on the e2e test? That way this PR can only look at changes to remix.
Comment thread.github/workflows/build.yml
Outdated

 uses: actions/setup-node@v3
 with:
 node-version-file: 'package.json'
 node-version: 18
@AbhiPrasadAbhiPrasadSep 6, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Can we read the e2e-tests package.json here instead?
@onurtemizkanonurtemizkan
mentioned this pull request
Sep 7, 2023
 Merged

@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
9b935d0 to
67c3c4eCompareSeptember 7, 2023 12:46
AbhiPrasad
AbhiPrasad
reviewed
Sep 8, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

One last thing before we merge - is there no way to get the remix version automatically? I would prefer not forcing folks to use the isRemixV2 flag.
@onurtemizkan
onurtemizkan
commented
Sep 8, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
One last thing before we merge - is there no way to get the remix version automatically? I would prefer not forcing folks to use the isRemixV2 flag.
Yes, I thought about it, there's no property including version available on client side (window.__remixContext), it's possible on server-side though. Is there a reliable way to pass that info on build time from server to client SDK?
Update: Passed remix version from server package.json to browser env while monkey-patching rootloader.
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
4 times, most recently
from
e62e166 to
09a1425CompareSeptember 11, 2023 18:54
@onurtemizkan
onurtemizkan
marked this pull request as draft
September 11, 2023 18:55
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
7 times, most recently
from
5eb3369 to
2ab1c11CompareSeptember 12, 2023 19:18
@huw
huw
commented
Sep 16, 2023•

edited

Loading

Copy link


Copy Markdown

Just FYI after upgrading my own setup:
  • error instanceof Error doesn’t work anymore for distinguishing between true client-side and ‘hydrated’ errors; we have to check for the presence of error.stack
  • Remix exports ErrorResponse from all runtimes and isRouteErrorResponse from @remix-run/react works fine for detection in all runtimes
  • ErrorResponses in handleError should probably therefore be sent to captureRemixServerException in the recommended setup (Remix now correctly emits them for Remix-internal errors that don’t end up in the error boundary; we just have to test for status >= 500)
  • captureRemixServerException should check for ErrorResponse instead of Response; I don’t think there are any situations where it would be sent a Response in Remix v2 but am much less sure of this than the others.
  • ErrorResponses that are sent to captureRemixServerException often include errorResponse.error, which is a private field in TypeScript but can be unwrapped to get an ordinary error to send to Sentry. This isn’t true for loader/action ErrorResponses.
Lms24
Lms24
reviewed
Sep 18, 2023
View reviewed changes
Comment threadpackages/remix/src/utils/futureFlags.ts
Outdated


Comment on lines

+50
 to 
+51

 const pkg = loadModule<{ version: string }>('@remix-run/react/package.json');
 const version = pkg ? pkg.version : '0.0.0';
@Lms24Lms24Sep 18, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

m: If we stick with this approach (see my other comment), let's add a debug log if we fail to load package.json
@sergical
sergical
commented
Sep 18, 2023

Copy link


Copy Markdown

Member
Remix V2 is out, would love to get this in 😅
@HazATHazAT
mentioned this pull request
Sep 18, 2023

Closed

@onurtemizkan
onurtemizkan
commented
Sep 19, 2023

Copy link


Copy Markdown

ContributorAuthor
Thanks for the trial @huw, it's been very helpful!
error instanceof Error doesn’t work anymore for distinguishing between true client-side and ‘hydrated’ errors; we have to check for the presence of error.stack
I was not able to reproduce this, but will take another look after the release.
ErrorResponses that are sent to captureRemixServerException often include errorResponse.error, which is a private field in TypeScript but can be unwrapped to get an ordinary error to send to Sentry. This isn’t true for loader/action ErrorResponses.
This may need a bit of discussion and new test cases.
We can revisit these two points as improvements in other PRs. The other 3 points @huw reported are now included in the first version.
Lms24
Lms24
approved these changes
Sep 19, 2023
View reviewed changes
@Lms24Lms24
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM now! I agree with merging this in now and tackling the open questions in follow up tasks/PRs. Thanks Onur!
Comment threadpackages/e2e-tests/test-applications/create-remix-app-v2/app/entry.client.tsx
Outdated

Comment threadpackages/e2e-tests/test-applications/create-remix-app-v2/playwright.config.ts
Outdated

@onurtemizkan@Lms24
Apply suggestions from code review
bf7ec96
Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
AbhiPrasad
AbhiPrasad
approved these changes
Sep 19, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Let's ship this as a first step - thanks Onur!
@AbhiPrasad
AbhiPrasad
merged commit 69b308d
into
developSep 19, 2023
@AbhiPrasad
AbhiPrasad
deleted the

onur/remix-v2-e2e-tests

branch
September 19, 2023 13:21
@chohnerchohner
mentioned this pull request
Sep 19, 2023
 Merged

2 tasks
@PrithvirajkumarPrithvirajkumar
mentioned this pull request
Sep 19, 2023

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@Lms24Lms24Lms24 approved these changes

+2 more reviewers

@huwhuwhuw left review comments
@AbhiPrasadAbhiPrasadAbhiPrasad approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Upgrade to remix v2

Make sure Remix SDK works without feature flags


5 participants

@onurtemizkan@huw@sergical@Lms24@AbhiPrasad

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
feat(remix): Add Remix 2.x release support. - #8940
Merged
AbhiPrasad merged 15 commits into
developfrom 
onur/remix-v2-e2e-tests
Sep 19, 2023
Merged
feat(remix): Add Remix 2.x release support.#8940
AbhiPrasad merged 15 commits into
developfrom 
onur/remix-v2-e2e-tests
Conversation
@onurtemizkan
@onurtemizkanonurtemizkan
commented
Sep 4, 2023•

edited by Lms24

Loading

Copy link


Copy Markdown

Contributor
Resolves: #8681
closes#9040
Summary:
  • Updated Node.JS version used on E2E tests to 18.x. (This is required for Remix v2.x, and 18.x is the current LTS so updated this for all) chore(e2e-tests): Use Node 18 for E2E tests. #8964
  • Updated create-remix-app E2E tests to cover client-side events and transactions.
  • Added create-remix-app-v2 E2E test application that uses 2.0.0 version.
Events:
  • Client-side captured event - Link
  • Client-side render error - Link
  • Server-side error - Link
  • pageload transaction - Link
  • navigation transaction - Link
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
6efcf2c to
0149461CompareSeptember 4, 2023 15:00
@github-actions
github-actionsBot
commented
Sep 5, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
size-limit report 📦
PathSize
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)75.58 KB (0%)
@sentry/browser (incl. Tracing) - Webpack (gzipped)31.49 KB (0%)
@sentry/browser - Webpack (gzipped)22.09 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)70.27 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)28.59 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped)20.66 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)222.15 KB (0%)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)86.64 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)61.49 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)31.47 KB (0%)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)75.61 KB (0%)
@sentry/react - Webpack (gzipped)22.12 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)93.49 KB (0%)
@sentry/nextjs Client - Webpack (gzipped)51.07 KB (0%)
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
2 times, most recently
from
abb124e to
b6ec1a3CompareSeptember 5, 2023 10:31
@onurtemizkanonurtemizkan
changed the title
feat(remix): Add flags for Remix v2 usage.feat(remix): Add flags for Remix 2.x usage.Sep 5, 2023
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
aef6350 to
ff4d574CompareSeptember 6, 2023 10:39
@onurtemizkan
onurtemizkan
marked this pull request as ready for review
September 6, 2023 11:31
AbhiPrasad
AbhiPrasad
reviewed
Sep 6, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Can we open up a separate PR for the node version changes on the e2e test? That way this PR can only look at changes to remix.
Comment thread.github/workflows/build.yml
Outdated

 uses: actions/setup-node@v3
 with:
 node-version-file: 'package.json'
 node-version: 18
@AbhiPrasadAbhiPrasadSep 6, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Can we read the e2e-tests package.json here instead?
@onurtemizkanonurtemizkan
mentioned this pull request
Sep 7, 2023
 Merged

@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
3 times, most recently
from
9b935d0 to
67c3c4eCompareSeptember 7, 2023 12:46
AbhiPrasad
AbhiPrasad
reviewed
Sep 8, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

One last thing before we merge - is there no way to get the remix version automatically? I would prefer not forcing folks to use the isRemixV2 flag.
@onurtemizkan
onurtemizkan
commented
Sep 8, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
One last thing before we merge - is there no way to get the remix version automatically? I would prefer not forcing folks to use the isRemixV2 flag.
Yes, I thought about it, there's no property including version available on client side (window.__remixContext), it's possible on server-side though. Is there a reliable way to pass that info on build time from server to client SDK?
Update: Passed remix version from server package.json to browser env while monkey-patching rootloader.
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
4 times, most recently
from
e62e166 to
09a1425CompareSeptember 11, 2023 18:54
@onurtemizkan
onurtemizkan
marked this pull request as draft
September 11, 2023 18:55
@onurtemizkan
onurtemizkanforce-pushed
the
onur/remix-v2-e2e-tests
branch
7 times, most recently
from
5eb3369 to
2ab1c11CompareSeptember 12, 2023 19:18
@huw
huw
commented
Sep 16, 2023•

edited

Loading

Copy link


Copy Markdown

Just FYI after upgrading my own setup:
  • error instanceof Error doesn’t work anymore for distinguishing between true client-side and ‘hydrated’ errors; we have to check for the presence of error.stack
  • Remix exports ErrorResponse from all runtimes and isRouteErrorResponse from @remix-run/react works fine for detection in all runtimes
  • ErrorResponses in handleError should probably therefore be sent to captureRemixServerException in the recommended setup (Remix now correctly emits them for Remix-internal errors that don’t end up in the error boundary; we just have to test for status >= 500)
  • captureRemixServerException should check for ErrorResponse instead of Response; I don’t think there are any situations where it would be sent a Response in Remix v2 but am much less sure of this than the others.
  • ErrorResponses that are sent to captureRemixServerException often include errorResponse.error, which is a private field in TypeScript but can be unwrapped to get an ordinary error to send to Sentry. This isn’t true for loader/action ErrorResponses.
Lms24
Lms24
reviewed
Sep 18, 2023
View reviewed changes
Comment threadpackages/remix/src/utils/futureFlags.ts
Outdated


Comment on lines

+50
 to 
+51

 const pkg = loadModule<{ version: string }>('@remix-run/react/package.json');
 const version = pkg ? pkg.version : '0.0.0';
@Lms24Lms24Sep 18, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

m: If we stick with this approach (see my other comment), let's add a debug log if we fail to load package.json
@sergical
sergical
commented
Sep 18, 2023

Copy link


Copy Markdown

Member
Remix V2 is out, would love to get this in 😅
@HazATHazAT
mentioned this pull request
Sep 18, 2023

Closed

@onurtemizkan
onurtemizkan
commented
Sep 19, 2023

Copy link


Copy Markdown

ContributorAuthor
Thanks for the trial @huw, it's been very helpful!
error instanceof Error doesn’t work anymore for distinguishing between true client-side and ‘hydrated’ errors; we have to check for the presence of error.stack
I was not able to reproduce this, but will take another look after the release.
ErrorResponses that are sent to captureRemixServerException often include errorResponse.error, which is a private field in TypeScript but can be unwrapped to get an ordinary error to send to Sentry. This isn’t true for loader/action ErrorResponses.
This may need a bit of discussion and new test cases.
We can revisit these two points as improvements in other PRs. The other 3 points @huw reported are now included in the first version.
Lms24
Lms24
approved these changes
Sep 19, 2023
View reviewed changes
@Lms24Lms24
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM now! I agree with merging this in now and tackling the open questions in follow up tasks/PRs. Thanks Onur!
Comment threadpackages/e2e-tests/test-applications/create-remix-app-v2/app/entry.client.tsx
Outdated

Comment threadpackages/e2e-tests/test-applications/create-remix-app-v2/playwright.config.ts
Outdated

@onurtemizkan@Lms24
Apply suggestions from code review
bf7ec96
Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
AbhiPrasad
AbhiPrasad
approved these changes
Sep 19, 2023
View reviewed changes
@AbhiPrasadAbhiPrasad
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Let's ship this as a first step - thanks Onur!
@AbhiPrasad
AbhiPrasad
merged commit 69b308d
into
developSep 19, 2023
@AbhiPrasad
AbhiPrasad
deleted the

onur/remix-v2-e2e-tests

branch
September 19, 2023 13:21
@chohnerchohner
mentioned this pull request
Sep 19, 2023
 Merged

2 tasks
@PrithvirajkumarPrithvirajkumar
mentioned this pull request
Sep 19, 2023

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@Lms24Lms24Lms24 approved these changes

+2 more reviewers

@huwhuwhuw left review comments
@AbhiPrasadAbhiPrasadAbhiPrasad approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Upgrade to remix v2

Make sure Remix SDK works without feature flags


5 participants

@onurtemizkan@huw@sergical@Lms24@AbhiPrasad