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

Apply promisify() to fs module functions to avoid error with Serverless v4 #1917

Merged

Conversation

ronkot
Copy link

@ronkot ronkot commented Sep 2, 2024

What did you implement:

Closes #1912

How did you implement it:

Instead of relying on some other party applying promisify on fs module functions, do it explicitly inside this library. Also, apply promisify in non-mutable way to keep fs module tidy.

This should fix the fs.readFileAsync is not a function error when used with serverless v4.

How can we verify it:

  • Unit tests are OK
  • Package some project with serverless v4 - process should not crash to the fs.readFileAsync is not a function error anymore
    • I tried to create serverless v4 example in the /examples folder but did not succeed. I think it's better if project maintainers would do that.

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

Copy link
Member

@j0k3r j0k3r left a comment

Choose a reason for hiding this comment

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

Looks ok to me.

I tried to created a Serverless v4 example as well, but it requires a licence key, so I stopped here...

@j0k3r j0k3r changed the title Apply promisify() to fs module functions (fixes #1912) Apply promisify() to fs module functions to avoid error with Serverless v4 Sep 6, 2024
@j0k3r j0k3r requested a review from vicary September 6, 2024 13:35
@j0k3r j0k3r added the bug label Sep 6, 2024
Copy link
Member

@vicary vicary left a comment

Choose a reason for hiding this comment

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

To put it lightly, v4 is not being easy on us plugin authors so far. Let's wait and see if we can actually cover these tests in the future.

The code looks fine and shouldn't do any harm at least, I'd say go ahead.

@j0k3r j0k3r merged commit aa74755 into serverless-heaven:master Sep 6, 2024
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError: fs.readFileAsync is not a function
3 participants