Skip to content

Conversation

@ilasw
Copy link
Contributor

@ilasw ilasw commented Mar 27, 2025

Changes

  • Added App folder support
  • Updated AuthContext to be used in app folder in future
  • Added useAppAuth hook for get boot data from the SSR queries
  • Added Funnel shared interfaces
  • Fixed failing tests

Events

Did you introduce any new tracking events?

Experiment

Did you introduce any new experiments?

Manual Testing

Caution

Please make sure existing components are not breaking/affected by this PR

Preview domain

https://feat-app-directory-helloworld.preview.app.daily.dev

@vercel
Copy link

vercel bot commented Mar 27, 2025

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

Name Status Preview Updated (UTC)
daily-webapp ✅ Ready (Inspect) Visit Preview Apr 3, 2025 9:37am
1 Skipped Deployment
Name Status Preview Updated (UTC)
storybook ⬜️ Ignored (Inspect) Apr 3, 2025 9:37am

Comment on lines -49 to -50
export const ACCEPTED_TYPES = 'image/png,image/jpeg';
export const acceptedTypesList = ACCEPTED_TYPES.split(',');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since they are used on server side code we should not mix with client-side one.

@ilasw ilasw changed the title feat: app directory and web funnel setup feat(funnels): app directory and web funnel setup Apr 3, 2025
Comment on lines 111 to 112
"jotai": "^2.12.2",
"jotai-tanstack-query": "^0.9.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are not using lets remove dependencies for now

Comment on lines +8 to +19
<meta
name="viewport"
content="initial-scale=1.0, width=device-width, viewport-fit=cover"
/>

<meta name="application-name" content="daily.dev" />
<meta name="apple-mobile-web-app-capable" content="yes" />
<meta name="apple-mobile-web-app-title" content="daily.dev" />
<meta name="format-detection" content="telephone=no" />
<meta name="mobile-web-app-capable" content="yes" />
<meta name="slack-app-id" content="A07AM7XC529" />
<meta name="apple-itunes-app" content="app-id=6740634400" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

But non blocking since I see you reused it so we don't have to update both places, we just need to make sure we move at some point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I can add a todo for this?

} => {
const params = useSearchParams();
return {
query: Object.fromEntries(params.entries()),
Copy link
Contributor

Choose a reason for hiding this comment

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

useMemo, params is stable so it should actually memoize quite nicely

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated ✔️

Comment on lines 29 to 33
const state = dehydrate(queryClient, { shouldDehydrateQuery: () => true }); // to also include Errors

if (!boot) {
throw new Error('Failed to fetch boot data');
}
Copy link
Contributor

Choose a reason for hiding this comment

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

fetchQuery will throw, so Promise will reject either way so you don't need this I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated ✔️

Copy link
Contributor

@rebelchris rebelchris left a comment

Choose a reason for hiding this comment

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

Nothing blocking from my side.
Just make sure to do critical flow testing once deployed just to be safe.

@@ -0,0 +1,36 @@
import type { Boot } from '@dailydotdev/shared/src/lib/boot';
Copy link
Member

@omBratteng omBratteng Apr 3, 2025

Choose a reason for hiding this comment

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

Slightly triggered by the file name app-boot.ts 🙈 (because of the hyphen)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed for the common wealth 😄

import React from 'react';
import Head from 'next/head';
import Providers from './providers';
import { AppHeadMetas } from '../../shared/src/features/common/components/AppHeadMetas';
Copy link
Member

Choose a reason for hiding this comment

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

Import via @dailydotdev?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done ✔️

Copy link
Member

@omBratteng omBratteng left a comment

Choose a reason for hiding this comment

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

LGTM – only two non-blocking comments

@ilasw ilasw merged commit e451dc3 into feat-web-funnel Apr 3, 2025
10 checks passed
@ilasw ilasw deleted the feat-app-directory-helloworld branch April 3, 2025 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants