Skip to content

fix: check type before using string-only method #2393

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

Merged
merged 10 commits into from
Dec 8, 2023
Merged

fix: check type before using string-only method #2393

merged 10 commits into from
Dec 8, 2023

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Dec 8, 2023

Description

Our override of fs/promises->readFile assumes that first argument is a string. readFile does accept multiple types - including URL that is used by Node.js itself (https://github.com/nodejs/node/blob/8cdb7cae98468efee222727ad9a4096e415f3ef6/lib/internal/modules/esm/load.js#L44-L46) when loading ESM module.

This adds typeguard to prevent trying to use .startsWith if argument is not a string. As we only care here about specific context of readFile usage - this continues to use a string, so we don't have to try to handle all the possible types that readFile accept and only need to handle actual types used in that context. If type is not string, we will do passthrough to original fs/promises.readFile function

Documentation

Tests

You can test this change yourself like so:

  1. TODO

Relevant links (GitHub issues, etc.) or a picture of cute animal

https://answers.netlify.com/t/all-nextjs-builds-returning-500-error-with-typeerror-file-startswith-is-not-a-function/107758/10

Copy link

netlify bot commented Dec 8, 2023

Deploy Preview for deft-cannoli-70cc77 canceled.

Name Link
🔨 Latest commit 66a26d7
🔍 Latest deploy log https://app.netlify.com/sites/deft-cannoli-70cc77/deploys/6573094ae405480007a3d6ec

Copy link

netlify bot commented Dec 8, 2023

Deploy Preview for netlify-plugin-nextjs-next-auth-demo ready!

Name Link
🔨 Latest commit 66a26d7
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-next-auth-demo/deploys/65730949d7b7fa0008ea9ea3
😎 Deploy Preview https://deploy-preview-2393--netlify-plugin-nextjs-next-auth-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 8, 2023

Deploy Preview for amazing-dieffenbachia-02ea4a ready!

Name Link
🔨 Latest commit 66a26d7
🔍 Latest deploy log https://app.netlify.com/sites/amazing-dieffenbachia-02ea4a/deploys/6573094aad904c0008bdc52d
😎 Deploy Preview https://deploy-preview-2393--amazing-dieffenbachia-02ea4a.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 8, 2023

Deploy Preview for netlify-plugin-nextjs-static-root-demo ready!

Name Link
🔨 Latest commit 66a26d7
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-static-root-demo/deploys/657309493a587e00087cce85
😎 Deploy Preview https://deploy-preview-2393--netlify-plugin-nextjs-static-root-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 8, 2023

Deploy Preview for netlify-plugin-nextjs-export-demo ready!

Name Link
🔨 Latest commit 66a26d7
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-export-demo/deploys/657309498316880008e43fb4
😎 Deploy Preview https://deploy-preview-2393--netlify-plugin-nextjs-export-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 8, 2023

Deploy Preview for next-plugin-canary ready!

Name Link
🔨 Latest commit 66a26d7
🔍 Latest deploy log https://app.netlify.com/sites/next-plugin-canary/deploys/65730949d4f7fc0008eda6b9
😎 Deploy Preview https://deploy-preview-2393--next-plugin-canary.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 8, 2023

Deploy Preview for next-plugin-edge-middleware ready!

Name Link
🔨 Latest commit 66a26d7
🔍 Latest deploy log https://app.netlify.com/sites/next-plugin-edge-middleware/deploys/657309490c6b6e000740c475
😎 Deploy Preview https://deploy-preview-2393--next-plugin-edge-middleware.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 8, 2023

Deploy Preview for netlify-plugin-nextjs-demo ready!

Name Link
🔨 Latest commit 66a26d7
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-demo/deploys/6573094994e2410008a24ba7
😎 Deploy Preview https://deploy-preview-2393--netlify-plugin-nextjs-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 8, 2023

Deploy Preview for netlify-plugin-nextjs-demo-all-flags ready!

Name Link
🔨 Latest commit 66a26d7
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-demo-all-flags/deploys/65730949e7be2c0008867828
😎 Deploy Preview https://deploy-preview-2393--netlify-plugin-nextjs-demo-all-flags.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 8, 2023

Deploy Preview for nextjs-plugin-custom-routes-demo ready!

Name Link
🔨 Latest commit 66a26d7
🔍 Latest deploy log https://app.netlify.com/sites/nextjs-plugin-custom-routes-demo/deploys/657309499fa6e50008eabe81
😎 Deploy Preview https://deploy-preview-2393--nextjs-plugin-custom-routes-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 8, 2023

Deploy Preview for next-i18next-demo ready!

Name Link
🔨 Latest commit 66a26d7
🔍 Latest deploy log https://app.netlify.com/sites/next-i18next-demo/deploys/65730949c8d93a0008259db1
😎 Deploy Preview https://deploy-preview-2393--next-i18next-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 8, 2023

Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo ready!

Name Link
🔨 Latest commit 66a26d7
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-nx-monorepo-demo/deploys/657309499ef3e20008695355
😎 Deploy Preview https://deploy-preview-2393--netlify-plugin-nextjs-nx-monorepo-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@pieh pieh changed the title Pieh patch 3 fix: check type before using string-only method Dec 8, 2023
@github-actions github-actions bot added the type: bug code to address defects in shipped code label Dec 8, 2023
@pieh pieh marked this pull request as ready for review December 8, 2023 11:03
@pieh pieh requested a review from a team as a code owner December 8, 2023 11:03
MarcL
MarcL previously approved these changes Dec 8, 2023
Copy link
Contributor

@MarcL MarcL left a comment

Choose a reason for hiding this comment

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

Looks good. I assume we're ok to bump the demos to Node 20.

@pieh pieh merged commit 06f0e0c into main Dec 8, 2023
@pieh pieh deleted the pieh-patch-3 branch December 8, 2023 12:58
pieh added a commit that referenced this pull request Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants