Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

chore: add astro to troubleshooting guide #3410

Merged
merged 7 commits into from
Feb 13, 2023

Conversation

reesscot
Copy link
Member

@reesscot reesscot commented Feb 10, 2023

Description of changes

Added instructions for Astro and updated instructions for Vite to be clearer. Also added both JS and TS version of the instructions and tested them out in an Astro app.

Issue #, if available

Related to: #3206 (comment)

Description of how you validated changes

Ran yarn docs dev and smoke tested the troubleshooting page:

CleanShot 2023-02-10 at 11 01 23

Checklist

  • PR description included
  • Relevant documentation is changed or added (and PR referenced)

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

@changeset-bot
Copy link

changeset-bot bot commented Feb 10, 2023

⚠️ No Changeset found

Latest commit: 3d558aa

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@reesscot reesscot temporarily deployed to ci February 10, 2023 18:01 — with GitHub Actions Inactive
@reesscot reesscot temporarily deployed to ci February 10, 2023 18:01 — with GitHub Actions Inactive
@reesscot reesscot temporarily deployed to ci February 10, 2023 18:01 — with GitHub Actions Inactive
@reesscot reesscot temporarily deployed to ci February 10, 2023 18:01 — with GitHub Actions Inactive
@reesscot reesscot marked this pull request as ready for review February 10, 2023 18:12
@reesscot reesscot requested a review from a team as a code owner February 10, 2023 18:12
window.global = window;
window.process = {
env: { DEBUG: undefined },
} as unknown as NodeJS.Process;
Copy link
Member Author

Choose a reason for hiding this comment

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

note: all astro files are TS files

@reesscot reesscot mentioned this pull request Feb 10, 2023
4 tasks
dbanksdesign
dbanksdesign previously approved these changes Feb 10, 2023
Copy link
Contributor

@dbanksdesign dbanksdesign left a comment

Choose a reason for hiding this comment

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

LGTM! Just one teeny comment

When working with a [Vite](https://vitejs.dev) project you must make a few modifications. Please follow the steps below.

**1.** Add the following script tag to the `index.html` file at the bottom of the `<body>` tag.
**1.** Add the following script tag to the `index.html` file right before the `</body>` tag.
Copy link
Contributor

Choose a reason for hiding this comment

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

Much more explicit!

@@ -56,10 +96,9 @@ When working with a [Vite](https://vitejs.dev) project you must make a few modif
</body>
```

**2.** Update the `vite.config.ts` and add in a resolve object inside the `defineConfig({})` as seen below.
**2.** Update the `vite.config.js` and add in a resolve alias inside the `defineConfig({})` as seen below.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we remove "in", "add in a resolve" v. "add a resolve"

@reesscot reesscot temporarily deployed to ci February 10, 2023 18:35 — with GitHub Actions Inactive
@reesscot reesscot temporarily deployed to ci February 10, 2023 18:35 — with GitHub Actions Inactive
@reesscot reesscot temporarily deployed to ci February 10, 2023 18:35 — with GitHub Actions Inactive
@reesscot reesscot temporarily deployed to ci February 10, 2023 18:35 — with GitHub Actions Inactive
Copy link
Contributor

@ErikCH ErikCH left a comment

Choose a reason for hiding this comment

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

I found this thread, and you can add in the window object in Vite as well, instead of adding it into the html file.

aws-amplify/amplify-js#9639 (comment)

export default defineConfig({
  integrations: [react(), tailwind()],
  vite: {
    optimizeDeps: {
      esbuildOptions: {
        define: {
          global: 'globalThis',
        },
      },
    },
    build: {
      rollupOptions: {
        plugins: [rollupNodePolyFill()],
      },
    },
    resolve: {
      alias: {
        './runtimeConfig': './runtimeConfig.browser',
      },
    },
  },
}) 

That's what I've done with Nuxt, which is also another section we should add eventually.

export default defineConfig({
  plugins: [react()],
  define: {
    global: {},
  },
  resolve: {
    alias: {
      "./runtimeConfig": "./runtimeConfig.browser",
    },
  },
});

@reesscot
Copy link
Member Author

I found this thread, and you can add in the window object in Vite as well, instead of adding it into the html file.

aws-amplify/amplify-js#9639 (comment)

export default defineConfig({
  integrations: [react(), tailwind()],
  vite: {
    optimizeDeps: {
      esbuildOptions: {
        define: {
          global: 'globalThis',
        },
      },
    },
    build: {
      rollupOptions: {
        plugins: [rollupNodePolyFill()],
      },
    },
    resolve: {
      alias: {
        './runtimeConfig': './runtimeConfig.browser',
      },
    },
  },
}) 

That's what I've done with Nuxt, which is also another section we should add eventually.

export default defineConfig({
  plugins: [react()],
  define: {
    global: {},
  },
  resolve: {
    alias: {
      "./runtimeConfig": "./runtimeConfig.browser",
    },
  },
});

I say that too. My concern there is that then you have to pull in an extra dependency rollupNodePolyFill to polyfill the rest of the Node globals (exports, and process). It feels a bit cleaner to do it all in one location, but I would be open to the idea of offering both options. Thoughts @ErikCH @dbanksdesign?

@reesscot reesscot temporarily deployed to ci February 10, 2023 22:26 — with GitHub Actions Inactive
@reesscot reesscot temporarily deployed to ci February 10, 2023 22:26 — with GitHub Actions Inactive
@reesscot reesscot temporarily deployed to ci February 10, 2023 22:26 — with GitHub Actions Inactive
@reesscot reesscot temporarily deployed to ci February 10, 2023 22:26 — with GitHub Actions Inactive
@ErikCH
Copy link
Contributor

ErikCH commented Feb 10, 2023

I say that too. My concern there is that then you have to pull in an extra dependency rollupNodePolyFill to polyfill the rest of the Node globals (exports, and process). It feels a bit cleaner to do it all in one location, but I would be open to the idea of offering both options. Thoughts @ErikCH @dbanksdesign?

I wasn't aware you had to pull in the pollyfill for it to work. In Nuxt case you don't have too. Then I'm fine with just adding it into the index.astro html file, and not adding the polyfill.

@reesscot
Copy link
Member Author

I say that too. My concern there is that then you have to pull in an extra dependency rollupNodePolyFill to polyfill the rest of the Node globals (exports, and process). It feels a bit cleaner to do it all in one location, but I would be open to the idea of offering both options. Thoughts @ErikCH @dbanksdesign?

I wasn't aware you had to pull in the pollyfill for it to work. In Nuxt case you don't have too. Then I'm fine with just adding it into the index.astro html file, and not adding the polyfill.

Yeah, unfortunately there are Auth flows where process and exports will throw runtime errors. It's a known issue and will be fixed in a future version of Auth.

@reesscot reesscot merged commit c5f9cb0 into main Feb 13, 2023
@reesscot reesscot deleted the reesscot-add-astro-troubleshooting branch February 13, 2023 20:05
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.

3 participants