Skip to content
This repository was archived by the owner on Feb 24, 2018. It is now read-only.

Update to use aws/aws-sdk-js#1123 explicit webpack support. #140

Merged
merged 1 commit into from
Sep 21, 2016

Conversation

simonbuchan
Copy link
Contributor

@simonbuchan simonbuchan commented Sep 9, 2016

aws/aws-sdk-js#1123 just merged and published version 2.6.0 adding native webpack support, which means we can simplify our suggested webpack setup.

  • Update README.md to use new aws-sdk webpack support.

  • Simplify NPM install by making the dependencies direct rather than peer dependencies.

    This is apparently OK for aws-sdk to do, so we should be able to may as well make it nicer
    for the end-user.

  • Remove old grunt build and deps.

The webpack setup docs could later be updated to talk about optimising bundle size, essentially, never require('aws-sdk') / import 'aws-sdk', always use aws-sdk/global or aws-sdk/clients/SERVICE:

var AWS = require('aws-sdk');
var S3 = AWS.S3;
// or
import AWS, { S3 } from 'aws-sdk';

AWS.config.region = 'us-west-1';
var s3 = new S3();

should instead be:

var AWS = require('aws-sdk/global');
var S3 = require('aws-sdk/clients/s3');
// or
import AWS from 'aws-sdk/global';
import S3 from 'aws-sdk/clients/s3';

But previous usage would still work as is so long as you update the aws-sdk you build your bundle from so the dist update for this should also update the aws-cognito-sdk.min.js build to be from 2.6.0

- Update README.md to use new aws-sdk webpack support.

- Simplify NPM install by making the dependencies direct.

  This is apparently OK for aws-sdk to do, so we may as well make it nicer
  for the end-user.

- Remove old grunt build and deps.
@simonbuchan simonbuchan changed the title WIP: Update to use aws/aws-sdk-js#1123 explicit webpack support. Update to use aws/aws-sdk-js#1123 explicit webpack support. Sep 9, 2016
@itrestian
Copy link
Contributor

So should we hold off on the merge for now or is it ok?

@simonbuchan
Copy link
Contributor Author

Sorry, I was still a bit unclear! Just got a reply for the import issue, but that is just for the suggested usage doc update.
Otherwise I think this is good.
The convention I've seen is to have "WIP" in title until the submitter thinks it's ready

@simonbuchan
Copy link
Contributor Author

@itrestian See edits to description, in particular this will require updating aws-cognito-sdk.min.js for script file users.

@borisirota
Copy link
Contributor

@itrestian Hi, any reason it isn't being merged ?

@itrestian
Copy link
Contributor

No reason! We will merge it but I need to do the things Simon mentioned: build aws-cognito-sdk.min.js from aws-sdk 2.6.0. And check if everything works correctly.

@borisirota
Copy link
Contributor

Great Thanks 👍

@itrestian
Copy link
Contributor

Can you confirm that we can use, modify, copy, and redistribute this PR? Thanks!

@simonbuchan
Copy link
Contributor Author

You can use, modify, copy, and redistribute this PR.

@simonbuchan
Copy link
Contributor Author

@itrestian Ping?

@itrestian
Copy link
Contributor

Yes, just started working on this :).

@itrestian
Copy link
Contributor

Hi Simon,

Everything works great for me. I updated aws-cognito-sdk.min.js from aws-sdk v2.6.4 and it seems to be working. I will merge ASAP!

Thanks a lot!

@itrestian itrestian merged commit 4e93bc9 into amazon-archives:master Sep 21, 2016
@itrestian
Copy link
Contributor

Merged and updated dist files.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants