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

Netlify Function Builder Detector for netlify-lambda can detect wrong functions source folder #1726

Closed
Robin-Hoodie opened this issue Jan 12, 2021 · 3 comments · Fixed by #1776
Labels
area: command: functions good first issue type: bug code to address defects in shipped code

Comments

@Robin-Hoodie
Copy link
Contributor

Robin-Hoodie commented Jan 12, 2021

Describe the bug

Having an NPM script which uses netlify-lambda and options that come after the functions source folder will incorrectly detect the option as the function source.

To Reproduce

Steps to reproduce the behavior:

  1. Set up a new Netlify repo with netlify-lambda as a dependency
  2. Add an NPM script, e.g. functions:build that runs netlify netlify-lambda build and passes an argument after specifying the functions source folder, e.g. netlify-lambda build functions/src -c ./functions/webpack.config.js
  3. Run netlify dev, which will pick up the functions:build script
  4. Notice that Netlify Dev logs "building functions from directory ./functions/webpack.config.js"
  5. Changes in the function source won't be picked up either, only changes to ./functions/webpack.config.js

Expected behavior

The functions source directory should be able to be detected regardless of which spot any options passed to netlify-lambda are passed

Additional context

As netlify-lambda build allows any options to be placed either before or after the functions source folder itself, that should also work when used together with Netlify Dev.

The problem should be quite easily fixed by modifying the regex used here

const match = script.match(/netlify-lambda build.* (\S+)\s*$/)

Happy to take a look at the regex and open a PR, just give me a shout

@Robin-Hoodie Robin-Hoodie added the type: bug code to address defects in shipped code label Jan 12, 2021
@Robin-Hoodie Robin-Hoodie changed the title Netlify dev function detect builder for netlify-lambda can detect wrong functions source folder Netlify function builder detector for netlify-lambda can detect wrong functions source folder Jan 12, 2021
@Robin-Hoodie Robin-Hoodie changed the title Netlify function builder detector for netlify-lambda can detect wrong functions source folder Netlify Function Builder Detector for netlify-lambda can detect wrong functions source folder Jan 12, 2021
@erezrokah
Copy link
Contributor

Thanks @Robin-Hoodie, we would love a contribution for this.
It would be probably be best to use a library to parse the arguments.
netlify-lambda uses commander. The CLI uses oclif but also has minimist as a dependency.

@Robin-Hoodie
Copy link
Contributor Author

Robin-Hoodie commented Jan 15, 2021

I've developed a fix for this, though while I was adding tests, it seems like the current code was intentionally written with scenarios in mind where no functions folder was provided to netlify-lambda build which you can see in this test

Is this something we want to keep support for? I can't imagine where it would be useful to run netlify-lambda build without a functions directory.

Edit

Just tried running netlify-lambda build -c ./functions/webpack.config.js
Which results in error: missing required argument 'dir'

I assume I can safely remove support (and the test) for this?

@erezrokah
Copy link
Contributor

Just tried running netlify-lambda build -c ./functions/webpack.config.js
Which results in error: missing required argument 'dir'

I assume I can safely remove support (and the test) for this?

Thanks @Robin-Hoodie, it would be great to get this fixed.

I believe the test's purpose is to explain the limitations of the current implementation. We can remove it or change the code to print a warning in that case (and change the test accordingly).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: command: functions good first issue type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants