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

Add @aws-amplify/ui-standalone package #394

Closed
wants to merge 7 commits into from
Closed

Conversation

ericclemmons
Copy link
Contributor

@ericclemmons ericclemmons commented Sep 17, 2021

Description of changes:

This was an attempt to use the Authenticator in a Vanilla JS context with aws-amplify, but this is blocked by aws-amplify/amplify-js#9639.

import {Authenticator} from '@aws-amplify/ui-standalone/authenticator';

const container = document.querySelector<HTMLDivElement>('#app')!;

new Authenticator(container);
  • Creates @aws-amplify/ui-standalone
  • Use @preact/compat in place of react and react-dom
  • Explicitly import * as React from 'react';this is required for aliasing to Preact to work!
  • Replace qrcode for React to something that's browser-friendly
  • Disabled autoprefixer because it's not browser-friendly

Screen Shot 2021-09-17 at 3 29 27 PM


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ericclemmons ericclemmons temporarily deployed to ci September 17, 2021 18:35 Inactive
@ericclemmons ericclemmons temporarily deployed to ci September 17, 2021 18:35 Inactive
@ericclemmons ericclemmons temporarily deployed to ci September 17, 2021 18:35 Inactive
@ericclemmons
Copy link
Contributor Author

Also, qrcode isn't ESM/Browser friendly:

Uncaught ReferenceError: Buffer is not defined

soldair/node-qrcode#215

import { Authenticator } from '@aws-amplify/ui-standalone';
import awsExports from '../../../environments/auth-with-username-no-attributes/src/aws-exports';

import '@aws-amplify/ui-react/styles.css';
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should either be:

  1. Style injected (even for the react package perhaps?)
  2. Or be re-exported from the ui-standalone package. Since usage of Authenticator from the standalone package should not involve pulling in something from ui-react

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re-export is the way we've been going 👍

Comment on lines +5 to +12
export class Authenticator {
constructor(container: HTMLElement) {
ReactDOM.render(
<AuthenticatorComponent>{() => <h1>Howdy</h1>}</AuthenticatorComponent>,
container
);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a great start! I am planning to make it more generic post this change gets merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm curious what the behavior should be render props for an imperative wrapper 🤔

Maybe:

new Authenticator(container).show().then(({ session, signOut, user }) => {
  alert(`Howdy ${user.username}`)
});

Honestly, the "Vanilla Authenticator" example is a bit contrived – I don't have much customer data on what the use-cases are for this specifically.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's pretty much the correct analogy. Treating the return value from the imperative wrapper as a ref and acting accordingly would perhaps be the right usage.

@ayush987goyal
Copy link
Contributor

As per our discussion, we could limit the scope of this package to say that it would only work with bundlers as described in the below doc too.
https://docs.amplify.aws/start/getting-started/setup/q/integration/js/

@ericclemmons
Copy link
Contributor Author

Closed in favor of #712

@wlee221 wlee221 deleted the standalone branch January 5, 2023 22:10
thaddmt pushed a commit that referenced this pull request Apr 7, 2023
Sync amplify-ui to amplify-ui-staging
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.

None yet

2 participants