Migrate aws-sdk from 2 to 3. Update rewrites, gatsby 5 optimisation #457

tractorcow · 2023-07-19T03:13:43Z

I have the basic logic and deployment working (with-redirects); I have a proof of concept deployed with a few issues.

Things I need to keep testing:

~~Update and run through testing.md~~ (fixed)
~~Resolve issue with client-only pages which work differently in gatsby 5 (they still get deployed to s3)~~ (fixed)
~~Test with serverless~~ (fixed)
~~Home page redirect gives a warning but still work fine~~ (fixed)

Notes from upgrade:

I've updated each dependency as far as could be done without breakage. I've been testing with gatsby 5 rather than trying to be as backwards-compatible as possible (but it should work fine with older gatsby versions).
I'm respecting both HTTP_PROXY and HTTPS_PROXY env vars, but I need to confirm how this should be documented.
I had a few issue with lerna so I ignored it for the most part. Instead a promoted a few dev dependencies from root to the plugin.
I've changed the way region detection works, but I'm not sure I'm 100% happy with the new logic.
I fixed a few things I think were bugs. E.g. maxRetries seems to have been ignored, and fixedRetryDelay seems to have been set via env not config.
I cleaned up as many linting / style errors as I could

The most significant update I've made is to do with rewrites:

generateRedirectObjectsForPermanentRedirects is now true by default. Turning this off breaks most rewrites, so I'm only testing for when this is enabled.
Home page redirects no longer uses fake-index.js, and is now a standard file rewrite. The existing solution had some issues, with some warnings (that ended up actually breaking some deployments).
Wildcard redirects from subpages to parents are disabled (e.g. /sub/* -> /sub) due to them causing infinite loops.
Wildcard patterns are not implemented at all (e.g. '[...]/index.ts`), outside of client-only paths. There is a note in readme on avoiding them.

With regards to release, it's my recommendation that you release a 1.0.0 under the current codebase, with legacy support for older gatsby versions, and tag this as 2.0.0 beta. I haven't tested this with gatsby 2,3. etc... and it should still work, but I have only tested this with gatsby 5, so you may want to target this version in order to prevent older sites immediately breaking on dependency updates.

tractorcow · 2023-07-20T04:52:08Z

Fixed up issues with client-only, serverless, and redirects. Need to get e2e testing running and green and we should be good. :)

tractorcow · 2023-07-24T05:27:07Z

I've come up with a nicer solution for home page redirects (no longer need a dummy js file) and I've implemented a TODO to flip generateRedirectObjectsForPermanentRedirects to true. I also addressed an issue with auto-created buckets.

Still working through e2e testing, but once this is working, I'll be ready to flip from draft to PR :)

tractorcow · 2023-07-25T02:57:06Z

Aha, great success

I think we are ready to go now?

tractorcow · 2023-07-25T03:08:14Z

@JoshuaWalsh can you please review

YoshiWalsh · 2023-07-25T04:15:21Z

This looks fantastic, thanks for putting the effort into this. My work and life are both very busy at the moment, but I'll try to make some time to review this. (Hopefully on Sunday at the latest)

tractorcow · 2023-07-25T04:39:27Z

It's ok. We actually critically depend on this module (as I expect many others do), so it's worth my time to invest in updating it. :)

gatsby-plugin-s3-e2e-tests-infrastructure/test-policy.json

zamboni86 · 2023-07-28T05:20:05Z

gatsby-plugin-s3-e2e-tests-infrastructure/template.tf

 }

 provider "aws" {
-    version = "~> 2.59"


this version should probably be kept as is or increased to something more recent!

The version was shifted to terraform.required_providers to upgrade to the new terraform style.

See https://registry.terraform.io/providers/hashicorp/aws/latest/docs#example-usage

ah! I missed that, this looks good then. My mistake.

YoshiWalsh · 2023-07-30T06:20:06Z

Hi @tractorcow , I tried to test this out today but I ran into a couple of issues.

\1. I'm unable to run npm ci in the root project, npm reports that package-lock.json is out of sync with package.json.

\2. I'm unable to build the project using npx lerna run build, I get the following error:

src/bin.ts(460,9): error TS7006: Parameter 'args' implicitly has an 'any' type.
       src/bin.ts(460,9): error TS2769: No overload matches this call.
         The last overload gave the following error.
           Argument of type '(args: any) => any' is not assignable to parameter of type '{ [key: string]: Options; }'.
             Index signature for type 'string' is missing in type '(args: any) => any'.

\3. I'm unable to build the gatsby-plugin-s3 project with npx tsc --project .. I get the following error:

src/bin.ts:460:9 - error TS7006: Parameter 'args' implicitly has an 'any' type.

460         args =>
            ~~~~

  The last overload gave the following error.
    Argument of type '(args: any) => any' is not assignable to parameter of type '{ [key: string]: Options; }'.
      Index signature for type 'string' is missing in type '(args: any) => any'.

460         args =>
            ~~~~~~~
461             args
    ~~~~~~~~~~~~~~~~
...
473                     // eslint-disable-next-line @typescript-eslint/no-explicit-any
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
474                 }) as any,
    ~~~~~~~~~~~~~~~~~~~~~~~~~

  ../node_modules/@types/yargs/index.d.ts:154:9
    154         command<O extends { [key: string]: Options }>(
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    155             command: string | ReadonlyArray<string>,
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    ...
    160             deprecated?: boolean | string,
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    161         ): Argv<T>;
        ~~~~~~~~~~~~~~~~~~~
    The last overload is declared here.


Found 2 errors in the same file, starting at: src/bin.ts:460

I'm not sure why running the build through Lerna vs directly yields different results. I did some additional tests and found that both methods definitely use the workspace version of TypeScript. (Not the globally installed version)

Please let me know if there's something I'm doing wrong. Thanks!

tractorcow · 2023-07-31T21:15:46Z

hi @JoshuaWalsh I gave you write access to the fork in case you wanted to push up any changes. I'll get back to you today on this.

tractorcow · 2023-07-31T21:49:23Z

Typescript errors have been fixed. My mistake was building at the module level, not using lerna.

tractorcow · 2023-08-01T21:33:16Z

I think there's still a minor issue where typescript 4 / 5 might be used in different build contexts. It still builds correctly in either case. :)

YoshiWalsh · 2023-08-02T12:46:38Z

I'm encountering a strange issue while trying to test this on Windows. The e2e script is failing to build the sample sites:

 building site gatsby-plugin-s3-example-with-redirects.
    success compile gatsby files - 0.787s

     ERROR #10123  API.CONFIG.LOADING

    We encountered an error while trying to load your site's gatsby-config. Please
    fix the error and try again.

      Error: Only URLs with a scheme in: file, data, and node are supported by the d
      efault ESM loader. On Windows, absolute paths must be valid file:// URLs. Rece
      ived protocol 'c:'

It's strange because if I manually run the same node command that buildSite is generating in the same working directory, it works perfectly. I'll take more of a look when I have a bit more time.

I did try modifying the buildSite function to run gatsby -v instead of gatsby build and it's definitely using 5.11.0, which has gatsbyjs/gatsby#37251 fixed.

tractorcow · 2023-08-03T04:58:22Z

Ah, full disclosure, I have been testing on OSX, but I can try to setup a windows environment to test at some point too. I'll have a look over the diffs to see if anything jumps out as a possible red flag.

tractorcow · 2023-08-03T04:59:41Z

Are you able to do a build on the gatsby-plugin-s3-example-with-redirects manually via npm run build in that folder? (you may need to use an .env file)

tractorcow · 2023-08-03T05:09:44Z

Here's something that might work. Try testing with node 18/20

https://stackoverflow.com/questions/69665780/error-err-unsupported-esm-url-scheme-only-file-and-data-urls-are-supported-by

My problem is because my Node.js version is too low. Upgrade to Node.js 16 solved the problem.

YoshiWalsh · 2023-08-06T12:49:35Z

I suspect the issue I'm having with gatsby-config isn't introduced by one of your code changes, but rather is caused by upgrading the Gatsby version. I am using Node 18, as the new Gatsby version required me to update.

npm run build in that directory works without any issues. I also tried modifying the runScript function to use spawn instead of fork, but got the same result. There must be some difference between the way Node is spawning the process and the way PowerShell does. Perhaps it's related to connecting stdio to pipes or something. I'll keep looking into it whenever I have time.

tractorcow · 2023-08-06T21:50:24Z

Thanks for giving it a good attempt; There may be some differences since I am on OSX and you are on windows, but at least between the two of us we can get a good OS testing coverage. :)

dan-lind · 2023-09-18T13:52:25Z

Any chance of moving this forward anytime soon @JoshuaWalsh? This looks like a great addition

tractorcow · 2023-09-18T20:47:17Z

Is there a way of doing a limited beta release that will let other users test and provide feedback? That might take some pressure off @JoshuaWalsh to do all the testing on his end.

dan-lind · 2023-09-19T07:50:15Z

Good idea, we would be happy to help test this!

Nantris · 2024-01-18T23:15:24Z

It seems a lot of effort was put into this PR. I hope a maintainer can help get this moved along.

PS: We don't need the change even - it's just a shame for good effort to go to waste.

afladmark · 2024-01-18T23:42:53Z

If it helps at all to have a user chime in, I've been running this branch live in production for about 2 months with no issues. Running against a ~3500 page Gatsby 4 deployment.

dan-lind · 2024-01-19T08:53:56Z

@YoshiWalsh You mentioned here #422 (comment)
that you discussed with jariz and took over as maintainer of the project. What needs to happen for this PR to make it into the master-branch?

tractorcow · 2024-01-22T04:04:55Z

I'm glad to hear others are finding this work helpful. I've also been using this on a few projects, but I understand that testing can be complicated.

I suggest that we split this into a new major version to minimise the risk of existing sites unexpectedly breaking, especially with the lack of regression testing on older gatsby versions (although happy to hear 4 works as well as 5).

Strictly this is a breaking change, so it would be good to have a good 1.0.0 (current) with 2.0.0 (this pr).

tractorcow · 2024-05-27T22:14:47Z

I made an attempt to publish this to https://www.npmjs.com/package/@pixelfusion-nz/gatsby-plugin-s3

Forgive me if I made a mistake, this is my first time publishing an npm package. :)

tractorcow · 2024-06-09T22:35:23Z

I've updated https://www.npmjs.com/package/@pixelfusion-nz/gatsby-plugin-s3 with a readme

npm i @pixelfusion-nz/gatsby-plugin-s3

I'll be testing this over the next week with some of my other projects to make sure I've published the package correctly.

tractorcow added 20 commits July 18, 2023 11:52

feat: process all npm dependency upgrades

add8640

chore: bump dep

5954b45

chore: update lock

7f8b12b

fix: update repo install instructions

73c77a0

feat: update bin.ts for aws-sdk v3

6cee425

fix: imports, code style, and linting

aaa39ac

fix: remove redundant option

c3eed46

fix: update proxy logic to support http / https

3a71606

fix: upgrade tests

47e0cc3

fix: linting in example

9e63afc

fix: adjust dependencies and build errors

208340b

fix: cleanup remaining issues with leftover aws-sdk references

dd18aab

chore: remove upgrade notes

57800cb

fix: e2e test and terraform

ba6df92

fix: correct check for no such bucket

e707ef0

chore: ignore env from source control

acb140e

fix: client only paths

9ef8278

fix: prevent infinite redirects

4fe4380

fix: linting

6c8dcb6

fix: serverless deployments

9b455a0

tractorcow added 3 commits July 24, 2023 15:20

fix: create bucket policy behaviour

22ccf2e

fix: use better solution for home page redirects

b30d275

fix: terraform permissions for creating buckets

3210b86

tractorcow added 4 commits July 25, 2023 11:44

fix: resolve infinite redirects, and document s3 limitations

2b9f266

fix: update unit tests

35b0e43

fix: missing permissions

f95d6d2

fix: correct policy for tests

9d1cb9c

YoshiWalsh self-requested a review July 25, 2023 04:19

zamboni86 reviewed Jul 28, 2023

View reviewed changes

gatsby-plugin-s3-e2e-tests-infrastructure/test-policy.json Outdated Show resolved Hide resolved

zamboni86 approved these changes Jul 28, 2023

View reviewed changes

tractorcow added 2 commits August 1, 2023 09:19

fix: sync package-lock.json with package.json

fb97120

fix: typescript errors

af6fb03

fix: remove redundant policy

fc0b2fa

dan-lind mentioned this pull request Sep 20, 2023

Please migrate your code to use AWS SDK for JavaScript (v3) #423

Open

pdcp1 mentioned this pull request Sep 21, 2023

Fix AWS SDK warning during build responsible-ai-collaborative/aiid#1985

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate aws-sdk from 2 to 3. Update rewrites, gatsby 5 optimisation #457

Migrate aws-sdk from 2 to 3. Update rewrites, gatsby 5 optimisation #457

tractorcow commented Jul 19, 2023 •

edited

tractorcow commented Jul 20, 2023

tractorcow commented Jul 24, 2023 •

edited

tractorcow commented Jul 25, 2023

tractorcow commented Jul 25, 2023

YoshiWalsh commented Jul 25, 2023

tractorcow commented Jul 25, 2023

zamboni86 Jul 28, 2023

tractorcow Jul 31, 2023

tractorcow Jul 31, 2023

zamboni86 Aug 1, 2023

YoshiWalsh commented Jul 30, 2023

tractorcow commented Jul 31, 2023

tractorcow commented Jul 31, 2023

tractorcow commented Aug 1, 2023

YoshiWalsh commented Aug 2, 2023 •

edited

tractorcow commented Aug 3, 2023

tractorcow commented Aug 3, 2023

tractorcow commented Aug 3, 2023

YoshiWalsh commented Aug 6, 2023 •

edited

tractorcow commented Aug 6, 2023

dan-lind commented Sep 18, 2023

tractorcow commented Sep 18, 2023

dan-lind commented Sep 19, 2023

Nantris commented Jan 18, 2024 •

edited

afladmark commented Jan 18, 2024

dan-lind commented Jan 19, 2024

tractorcow commented Jan 22, 2024

tractorcow commented May 27, 2024

tractorcow commented Jun 9, 2024

Migrate aws-sdk from 2 to 3. Update rewrites, gatsby 5 optimisation #457

Are you sure you want to change the base?

Migrate aws-sdk from 2 to 3. Update rewrites, gatsby 5 optimisation #457

Conversation

tractorcow commented Jul 19, 2023 • edited

tractorcow commented Jul 20, 2023

tractorcow commented Jul 24, 2023 • edited

tractorcow commented Jul 25, 2023

tractorcow commented Jul 25, 2023

YoshiWalsh commented Jul 25, 2023

tractorcow commented Jul 25, 2023

zamboni86 Jul 28, 2023

Choose a reason for hiding this comment

tractorcow Jul 31, 2023

Choose a reason for hiding this comment

tractorcow Jul 31, 2023

Choose a reason for hiding this comment

zamboni86 Aug 1, 2023

Choose a reason for hiding this comment

YoshiWalsh commented Jul 30, 2023

tractorcow commented Jul 31, 2023

tractorcow commented Jul 31, 2023

tractorcow commented Aug 1, 2023

YoshiWalsh commented Aug 2, 2023 • edited

tractorcow commented Aug 3, 2023

tractorcow commented Aug 3, 2023

tractorcow commented Aug 3, 2023

YoshiWalsh commented Aug 6, 2023 • edited

tractorcow commented Aug 6, 2023

dan-lind commented Sep 18, 2023

tractorcow commented Sep 18, 2023

dan-lind commented Sep 19, 2023

Nantris commented Jan 18, 2024 • edited

afladmark commented Jan 18, 2024

dan-lind commented Jan 19, 2024

tractorcow commented Jan 22, 2024

tractorcow commented May 27, 2024

tractorcow commented Jun 9, 2024

tractorcow commented Jul 19, 2023 •

edited

tractorcow commented Jul 24, 2023 •

edited

YoshiWalsh commented Aug 2, 2023 •

edited

YoshiWalsh commented Aug 6, 2023 •

edited

Nantris commented Jan 18, 2024 •

edited