Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: add Gen2 specific doc for using SSR adapter with Nuxt 3 #7575

Merged
merged 9 commits into from May 20, 2024
Merged

chore: add Gen2 specific doc for using SSR adapter with Nuxt 3 #7575

merged 9 commits into from May 20, 2024

Conversation

HuiSF
Copy link
Contributor

@HuiSF HuiSF commented May 11, 2024 •
edited

Description of changes:

Ported the Nuxt 3 doc from the Gen1 section with:

  1. Replaced amplifyconfiguration.json with amplify_outputs.json
  2. Replaced Gen1 GraphQL client with Gen2 Typebeast Data client
  3. Updated Storage example to use the new path parameters instead of accessLevel
  4. Some minor cleanups
  5. Removed the accordion wrappers from code examples (feedback from @josefaidt)

Related GitHub issue #, if available:

Instructions

If this PR should not be merged upon approval for any reason, please submit as a DRAFT

Which product(s) are affected by this PR (if applicable)?

  • amplify-cli
  • amplify-ui
  • amplify-studio
  • amplify-hosting
  • amplify-libraries

Which platform(s) are affected by this PR (if applicable)?

  • JS
  • Swift
  • Android
  • Flutter
  • React Native

Please add the product(s)/platform(s) affected to the PR title

Checks

  • Does this PR conform to the styleguide?

  • Does this PR include filetypes other than markdown or images? Please add or update unit tests accordingly.

  • Are any files being deleted with this PR? If so, have the needed redirects been created?

  • Are all links in MDX files using the MDX link syntax rather than HTML link syntax?

    ref: MDX: [link](https://docs.amplify.aws/)
    HTML: <a href="https://docs.amplify.aws/">link</a>

When this PR is ready to merge, please check the box below

  • Ready to merge

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@HuiSF HuiSF requested a review from ashika112 May 11, 2024 00:24
@HuiSF HuiSF requested a review from iartemiev May 13, 2024 17:50
ashika112
ashika112 previously approved these changes May 13, 2024
View reviewed changes
josefaidt
josefaidt reviewed May 13, 2024
View reviewed changes
src/pages/[platform]/build-a-backend/server-side-rendering/nuxt/index.mdx Outdated
Comment on lines 156 to 157
// Extra the model operation function types for creating wrapper function later
type RemoveFirstParam<Params extends any[]> = Params extends [infer _, ...infer Rest] ? Rest : never;
Copy link
Contributor

Choose a reason for hiding this comment

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

can this be removed now in favor of Schema["SomeModel"]["type"]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The model operation APIs have a second parameter as well as options, Schema["SomeModel"]["type"] is insufficient for this use case.

Copy link
Contributor

Choose a reason for hiding this comment

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

For future ref: @svidgen is working on a refactor that will expose additional CRUDL input types directly on the Schema type. E.g. you'll be able to call Schema['ModelName']['createType']. Full list here. Let us know if this will solve your intended use case.

josefaidt
josefaidt reviewed May 13, 2024
View reviewed changes
src/pages/[platform]/build-a-backend/server-side-rendering/nuxt/index.mdx
} from 'aws-amplify/adapter-core';
import { parseAmplifyConfig } from 'aws-amplify/utils';

import type { LibraryOptions } from '@aws-amplify/core';
Copy link
Contributor

Choose a reason for hiding this comment

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

same nit for grouping type imports at the top of the file. Additionally, does this break when using npm's install-strategy=linked, yarn, or pnpm? pnpm will not resolve imports from transient deps

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested with install-strategy=linked Nuxt itself started failing while running postinstall script. Consider the following statement printed by npm

npm WARN reify The "linked" install strategy is EXPERIMENTAL and may contain bugs.

I think we can use this import path as is for now.

Copy link
Contributor

@josefaidt josefaidt May 16, 2024 •
edited

Choose a reason for hiding this comment

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

is this type export available from the top-level package? pnpm is unable to resolve it
CleanShot 2024-05-16 at 13 02 33@2x

https://web.archive.org/web/20240229022602/https://broadcrunch.com/technology/computing/phantom-dependencies-in-nodejs-and-how-pnpm-prevents-them/

josefaidt
josefaidt reviewed May 13, 2024
View reviewed changes
src/pages/[platform]/build-a-backend/server-side-rendering/nuxt/index.mdx

Example implementation:

```ts title="middleware/amplifyAuthMiddleware.ts"
Copy link
Contributor

Choose a reason for hiding this comment

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

does Nuxt/Vue have a pattern for filenames? filenames earlier in the doc are kebab-case where this is camelCase

Copy link
Contributor Author

@HuiSF HuiSF May 13, 2024 •
edited

Choose a reason for hiding this comment

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

I don't think so, the snake cases above followed Nuxt documentations, tested using camel case doesn't break them. I can update and unify.

Files for renderings a actual route, have names in snake case following url formatting convention, e.g. /storage-list.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah I see it's back and forth in the docs. Unification would be nice for the consistency

CleanShot 2024-05-16 at 13 05 56@2x

@HuiSF @ashika112
Update src/pages/[platform]/build-a-backend/server-side-rendering/nux…
5251bea 
…t/index.mdx

Co-authored-by: ashika112 <155593080+ashika112@users.noreply.github.com>
@HuiSF HuiSF requested review from josefaidt and ashika112 May 13, 2024 23:42
iartemiev
iartemiev previously approved these changes May 14, 2024
View reviewed changes
Copy link
Contributor

@iartemiev iartemiev left a comment

Choose a reason for hiding this comment

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

LGTM. Left some nit suggested changes on wording etc. but feel free to ignore them. Just stuff that jumped out at me.

src/pages/[platform]/build-a-backend/server-side-rendering/nuxt/index.mdx Outdated Show resolved Hide resolved
src/pages/[platform]/build-a-backend/server-side-rendering/nuxt/index.mdx Outdated Show resolved Hide resolved
src/pages/[platform]/build-a-backend/server-side-rendering/nuxt/index.mdx Outdated Show resolved Hide resolved
src/pages/[platform]/build-a-backend/server-side-rendering/nuxt/index.mdx Outdated Show resolved Hide resolved
src/pages/[platform]/build-a-backend/server-side-rendering/nuxt/index.mdx Outdated Show resolved Hide resolved
src/pages/[platform]/build-a-backend/server-side-rendering/nuxt/index.mdx Outdated Show resolved Hide resolved
src/pages/[platform]/build-a-backend/server-side-rendering/nuxt/index.mdx Outdated
Comment on lines 156 to 157
// Extra the model operation function types for creating wrapper function later
type RemoveFirstParam<Params extends any[]> = Params extends [infer _, ...infer Rest] ? Rest : never;
Copy link
Contributor

Choose a reason for hiding this comment

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

For future ref: @svidgen is working on a refactor that will expose additional CRUDL input types directly on the Schema type. E.g. you'll be able to call Schema['ModelName']['createType']. Full list here. Let us know if this will solve your intended use case.

src/pages/[platform]/build-a-backend/server-side-rendering/nuxt/index.mdx Outdated Show resolved Hide resolved
src/pages/[platform]/build-a-backend/server-side-rendering/nuxt/index.mdx Outdated Show resolved Hide resolved
@HuiSF @iartemiev
Apply suggestions from code review
d7029ae 
Co-authored-by: Ivan Artemiev <29709626+iartemiev@users.noreply.github.com>
@renebrandel renebrandel enabled auto-merge (squash) May 20, 2024 16:28
@renebrandel renebrandel merged commit 402d3b4 into main May 20, 2024
10 checks passed
@renebrandel renebrandel deleted the hui/feat/gen2-nuxt-3 branch May 20, 2024 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@renebrandel renebrandel renebrandel approved these changes

@josefaidt josefaidt josefaidt approved these changes

@aspittel aspittel aspittel approved these changes

@iartemiev iartemiev iartemiev approved these changes

@ashika112 ashika112 ashika112 approved these changes

@ykethan ykethan Awaiting requested review from ykethan

@Jay2113 Jay2113 Awaiting requested review from Jay2113

@chrisbonifacio chrisbonifacio Awaiting requested review from chrisbonifacio

@swaminator swaminator Awaiting requested review from swaminator swaminator is a code owner

@dbanksdesign dbanksdesign Awaiting requested review from dbanksdesign dbanksdesign is a code owner

@ErikCH ErikCH Awaiting requested review from ErikCH

@arundna arundna Awaiting requested review from arundna

@hdworld11 hdworld11 Awaiting requested review from hdworld11

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@HuiSF @renebrandel @josefaidt @aspittel @iartemiev @ashika112