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

transformHeaders does not pass the path parameter #275

Closed
techfg opened this issue Sep 7, 2023 · 8 comments · Fixed by #276
Closed

transformHeaders does not pass the path parameter #275

techfg opened this issue Sep 7, 2023 · 8 comments · Fixed by #276

Comments

@techfg
Copy link
Contributor

techfg commented Sep 7, 2023

According to the docs, transformHeaders should provide the path parameter when invoked, however it is not currently passed (only the header values array is passed).

transformHeaders: (headers, path) => headers,

The path is required for special case handling of headers during transformation and the parameters should match the docs.

@jfo8001
Copy link

jfo8001 commented Sep 11, 2023

This is an URGENT blocker to gatsby cloud porting, please fix

@techfg
Copy link
Contributor Author

techfg commented Sep 12, 2023

The deadline for some customers to move from Gatsby Cloud to Netlify is 9/29. Unfortunately, there are many issues with Gatsby Adapters so being able to have same functionality with gatsby-plugin-netlify as GC customers did with gatsby-plugin-gatsby-cloud is critical to make the transition smoother.

Any chance someone can review #276?

FYI - For anyone moving to Netlify from Gatsby cloud and planning on using gatsby-plugin-netlify, please read this issue and choose one of the workarounds presented or your build will not use gatsby-plugin-netlify if you build on Netlify.

@isobel-taylor
Copy link

Thank you for putting up the fix. I hope it can be merged soon - this is a blocker for our migration.

@isobel-taylor
Copy link

isobel-taylor commented Sep 21, 2023

Netlify have stated they are not going to fix this issue. Consider this plugin unmaintained - they've no interest in supporting customers on Gatsby versions lower than 5.

@techfg
Copy link
Contributor Author

techfg commented Sep 25, 2023

Hi @isobel-taylor, thanks for the update although I'm hopeful that Netlify reconsiders their position on this.

Netlify folks -

Completely understand the preference to retire gatsby-plugin-netlify in favor of Gatsby Adapters (gatsby-adapter-netlify) on a go-forward basis. Unfortunately, Adapters have many issues and are lacking features that gatsby-plugin-netlify offered. Until Adapters have feature parity and until the issues with Adapters are resolved, customers may still require gatsby-plugin-netlify depending on their situation/use-cases. In short, using Adapters is simply not an option for some customers currently.

Most notably, this effects Gatsby Cloud customers migrating to Netlify that were using the transformHeaders feature of gatsby-plugin-gatsby-cloud and relying on the path parameter being passed.

Further complicating the issues are that Gatsby forces gatsby-adapter-netlify and removes gatsby-plugin-netlify on any builds performed on Netlify with Gatsby v5.12.0 or greater.

To summarize:

  1. Adapters are not in a usable state for some customers as there are multiple issues (e.g., bugs, feature parity) with them
  2. For customers migrating from Gatsby cloud that were using gatsby-plugin-gatsby-cloud and relying on path parameter:
    1. Gatsby < v5.12.0 - The only "smooth" migration path is to use gatsby-plugin-netlify with add path to transformHeaders - resolves #275 #276 merged
    2. Gatsby >= v5.12.0 - The only "smooth" migration path is a two step process:
      1. Disable adapters via the workaround provided here; OR have Gatsby PR 38549 merged and configure Gatsby appropriately
      2. Have add path to transformHeaders - resolves #275 #276 merged

Once Adapters have feature parity and the issues resolved, retiring gatsby-plugin-netlify makes sense but in the meantime, hopeful you will re-consider #276 to remove this blocker which will provide a smoother transition from Gatsby Cloud to Netlify.

Thank you!

@kodiakhq kodiakhq bot closed this as completed in #276 Sep 25, 2023
kodiakhq bot pushed a commit that referenced this issue Sep 25, 2023
Co-authored-by: Matt Kane <matt.kane@netlify.com>
@ascorbic
Copy link
Contributor

Thanks @techfg for raising this. I have merged the PR. We should be able to get the release out soon.

@isobel-taylor I'm sorry if you got the impression that we're not fixing things in this plugin. That's absolutely not the case, and we will still maintain it for as long as we need to support older versions of Gatsby.

Re disabling adapter: we're investigating the best immediate approach there - whether it's ensuring feature parity (which is the ideal), or just allowing them to be disabled. We will get back to you all with updates once we've worked out the best approach.

Thanks everyone for your patience.

@ascorbic
Copy link
Contributor

The fix for this is live in version 5.1.1. Thanks again @techfg

@techfg
Copy link
Contributor Author

techfg commented Sep 26, 2023

@ascorbic -

Thanks for reviewing and merging, great news! Glad to hear that plans are in the works to gain feature parity and/or provide an official way to disable adapters.

FWIW, along with addressing the issues with adapters, gaining feature parity would provide most customers a zero-config approach (one of the goals of adapters) while having an official way to disable also would serve those that either need more control over their build process or are just simply building on Netlify but deploying elsewhere (that doesn't have an adapter).

While that all gets worked out and with the first deadline approaching quickly (9/29) for Gatsby Cloud to Netlify migration with others sooner to follow, any idea timeline wise on when we might hear more on the final plans?

Thanks again!

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

Successfully merging a pull request may close this issue.

4 participants