Clarify/improve support for pure JS (no TS) projects

[hheimbuerger]

It's never explicitly stated either way, but I read the readme as implying that pure JS projects (not using TypeScript or Babel) are supported by this library:

• "/// entry point, index.js/ts"
• "For JS projects that cant use [...], just use equivalent [...]"

That said, I had no luck getting it working. I've tried with different versions of node, but I'm getting fairly a different (long) list of errors each time. Mostly SyntaxErrors on import statements, which made me think that it's a JS vs. TS (or rather: expectation of full ES6 support) situation. However, I also got a MODULE_NOT_FOUND for unicode-13.0.0 and a SyntaxError: Invalid regular expression: /[_.\- ]+([\p{Alpha}\p{N}_]|$)/: Invalid escape in funcNameFromRelPathDefault() of node_modules\better-firebase-functions\lib\index.js:1:40863.

This is after creating a new project (firebase init), not selecting TS, creating the following index.js and then running firebase functions:shell.

const bff = require('better-firebase-functions');
bff.exportFunctions({ __filename, module.exports });

Am I doing something wrong? Or is TypeScript simply a requirement to use better-firebase-functions? Then I would appreciate if that could be clarified in the readme.

If it is supported, then I would appreciate basic instructions for setting up a new project that way.

[george43g]

There are a few issues with the above code snippet. You've used the object shorthand without properly naming the property. For example {__filename, exports} is correct. In you case, however, it would have to be {__filename, exports: module.exports} since the package is looking for a property with the name exports on the config object.

Try reading through this tutorial as well: https://medium.com/@george43g/organise-firebase-functions-part-1-optimize-cold-start-time-cd36f76d2133

The package supports both JS and TS. Let me know if the above fixes your issue, if not, happy to help.

[hheimbuerger]

Thanks for the clarification on the property naming! Wouldn't I have to name the __filename as well, though?

Either way, I unfortunately didn't make any progress. I feel like I'm making another newbie mistake here, but I just can't figure it out right now.

Going with a concrete example, here's what I did:

1. Clear the npm cache
2. Install Node.js v8.17.0 (as dictated by Firebase) from scratch
3. Run npm install -g firebase-tools
4. Run firebase init
5. Choose a JS project with functions only
6. Run npm install --save better-firebase-functions and finally
7. Add to the index.js: require('better-firebase-functions').exportFunctions({__filename, exports: module.exports});

I end up with this very trivial test project: bff-js-example-v1.zip

Running firebase functions:shell gives me:

 SyntaxError: Invalid regular expression: /[_.\- ]+([\p{Alpha}\p{N}_]|$)/: Invalid escape
    at r (functions\node_modules\better-firebase-functions\lib\index.js:1:13745)
    at funcNameFromRelPathDefault (functions\node_modules\better-firebase-functions\lib\index.js:1:40863)
    at Module.exportFunctions (functions\node_modules\better-firebase-functions\lib\index.js:1:42521)
    at Object.<anonymous> (functions\index.js:4:5)

The issue seems to be the Unicode property escapes \p{...}, which according to MDN: RegExp require Node.js 10+.

So apparently the problem is that I'm running code directly in Node.js 8, which should have been Babel-transpiled?

[george43g]

Ahh, I think I know what's happening now. The library has been compiled to support node 10.15.3, which is the newer version supported by Firebase Functions. So it doesn't work in node 8...

I decided to release a branch for node 8. That way, to include it in your project, you just need to specify the branch on npm install , like so: npm install git@github.com:gramstr/better-firebase-functions.git#node8

Upon doing some further work, I was able to reproduce your error. It happens when using node 8. some of the bundled dependencies - including camelCase - are not compatible with node 8. I have put together a version (by downgrading some of the package dependencies) that should work on node 8 on the node8 branch. Try it out and let me know if your issue is fixed.

[hheimbuerger]

Ah, I don't actually mind switching to the Node.js 10 beta, I just wasn't aware Firebase already supports it.

I quickly tried your Node 8 branch, but it cannot be built out-of-the-box on Windows (its build command calls on rm). Would be easy enough to work around, but as I said, I don't actually care about staying on Node.js 8, so I switched over to 10 instead.

Now I'm back to the long list of errors that originally made me think there's a TS requirement here:

>  functions\node_modules\@firebase\component\dist\index.esm.js:1
>  import { __assign, __values, __read, __awaiter, __generator } from 'tslib';
>         ^
>
>  SyntaxError: Unexpected token {
>      at Module._compile (internal/modules/cjs/loader.js:723:23)
>      at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
>      at Module.load (internal/modules/cjs/loader.js:653:32)
>      at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
>      at Function.Module._load (internal/modules/cjs/loader.js:585:3)
>      at Module.require (internal/modules/cjs/loader.js:692:17)
>      at require (internal/modules/cjs/helpers.js:25:18)
>      at Module.exportFunctions (functions\node_modules\better-firebase-functions\lib\index.js:1:42883)
>      at Object.<anonymous> (functions\index.js:3:38)
>      at Module._compile (internal/modules/cjs/loader.js:778:30)
>  functions\node_modules\@firebase\component\dist\index.esm2017.js:1

This is with the index.js containing only:

require('better-firebase-functions').exportFunctions({__filename, exports: module.exports});

Is that still the wrong call for a pure JS project?

[george43g]

Strange. bff doesn't even rely on any firebase dependencies, its job is pretty much done with lodash. You don't need to require firebase-functions in your index file.

Can you paste the output of BFF in here so I can take a look and make sure your other config values are correct? Try the following:

const result = bff.exportFunctions(...) <- with your config And then paste the result of: console.log(result)

And then also try with exportPathMode set to true in your config: const result = bff.exportFunctions({...yourConfig, exportPathMode: true}); console.log(result)

[hheimbuerger]

Well, I cannot call exportFunctions() right now, at least with my (possibly incorrect?) configuration, or I get the above stream of syntax errors. I really don't have any additional files. I didn't define any functions, it's just the scaffolding generated by firebase init. My index.js also contains nothing else but

require('better-firebase-functions').exportFunctions({__filename, exports: module.exports});

I'm not importing firebase-functions or anything else.

bff-js-example-v2.zip

Regarding the import itself:

console.log(require('better-firebase-functions'));

gives me

>  Object [Module] {
>    exportFunctions: [Getter],
>    funcNameFromRelPathDefault: [Getter],
>    default: [Getter] }

[hheimbuerger]

You know, I just realized that the final error message comes from the Firebase CLI:

!  functions: Maximum call stack size exceeded
!  Your function was killed because it raised an unhandled error.

Is it possible that -- due to my configuration being incorrect -- bff is trying to load the entire node_modules directory, thinking those were functions!?

[hheimbuerger]

Could this be a Windows incompatibility? Caused by the backslashes in __filename, maybe?

[hheimbuerger]

Pretty much confirmed. I can 'fix' this by specifying a functionDirectoryPath one level below, so that the node_modules isn't contained in it (as the default of . does).

However, this just generally doesn't seem to be a supported way to run them, as it starts naming my functions e.g. http\foo, and I'm not even sure if it's possible to call those, etc.

To be honest, I'm not even sure if the Firebase emulators are officially supporting Windows. It just always worked without a hitch, so I didn't even consider running them in a Linux shell instead. So I think we can just close this as 'dev environment on Windows not supported'. No real use in supporting this, as production environments will always be Linux-based anyway.

Sorry, got really sidetracked by the misleading error messages there. But it should have tipped me off from the beginning that I got hundreds of stack traces, instead of a single one. I feel quite bad for wasting your time with this!

[george43g]

That's all good! I'm happy to receive people's feedback and improve my own skills in the process. I hope to make this package as useful as possible.

You can do everything with this package you would normally do with cloud functions/CLI. I am releasing a few more tools and tutorials to show people how to bundle the functions using webpack. Will also check out rollup and parcel.

The naming convention for functions is to use a dash - for export groups. The final exports object from the index file is allowed to have nested functions. A dash - denotes a group. These functions can be called remotely, etc... as you would with any other function.

Glad you got this working. It might be worth setting some smart defaults to prevent node_modules from being searched if it happens to be in the same dir as the index file.

[hheimbuerger]

The naming convention for functions is to use a dash - for export groups.

So just to clarify: the problem is that on Windows, it doesn't! It uses \ for export groups instead. The example function would be called http\new-user then. As the backslash is kind of a reserved character in most situations, this makes it a bit tricky to call them. (I assume it's possible somehow, otherwise Firebase probably wouldn't allow you to use it, but that's where I gave up with this library and went back to my copy-pasted function dispatcher.)

Glad you got this working. It might be worth setting some smart defaults to prevent node_modules from being searched if it happens to be in the same dir as the index file.

Ah, so that isn't a Windows-specific issue? In the default JS (not TS) configuration, the index.js does lie right next to the node_modules directory, so I would definitely recommend you exclude this by default!

Also, I'm wondering if there's a better way to detect this type of situation and abort earlier, without printing hundreds of stacktraces, which can be a bit confusing. Is it intended behavior to keep loading more (what it assumes to be) functions, even though the previous one failed to load? I'm wondering if the first failure should abort the process and not try to keep loading the other functions.

[george43g]

The naming convention for functions is to use a dash - for export groups.

So just to clarify: the problem is that on Windows, it doesn't! It uses \ for export groups instead. The example function would be called http\new-user then.

This is a bit confusing for me. So, the dash is how firebase denotes a subgroup. So on the module.exports object, which is what the Firebase CLI reads on deployment (and cold start), any particular key can either hold an actual function trigger or another object, which itself holds function triggers. The name of each function depends on the string which is the key for the trigger on the object. The dashes denote the path to the function trigger via the nested objects. For example, { auth: { onCreate: func, onDelete: func2 }} would result in auth-onCreate as the name for func, and auth-onDelete as the name for func2.

Which is why I can't understand how or why this would be different on windows. I think maybe you're confusing the directory path with the object path. The function name is based on the object path. The package simply tries to model the object to mirror the directory structure of your function trigger modules.

Do you think all of this needs further clarification in the guide and docs?

As the backslash is kind of a reserved character in most situations, this makes it a bit tricky to call them. (I assume it's possible somehow, otherwise Firebase probably wouldn't allow you to use it, but that's where I gave up with this library and went back to my copy-pasted function dispatcher.)

The same backslash denotes directories on both windows and Unix based systems, I believe. The only difference is the first part of the path, which starts with C:/... oh. Maybe that's what's breaking it.

Can you do me a favor and run console.log(__filename) on your windows system and paste me the output? There might be a chance that an error is caused when dealing with absolute paths (although I still don't see why...) on a windows system with a drive letter.

Glad you got this working. It might be worth setting some smart defaults to prevent node_modules from being searched if it happens to be in the same dir as the index file.

Ah, so that isn't a Windows-specific issue? In the default JS (not TS) configuration, the index.js does lie right next to the node_modules directory, so I would definitely recommend you exclude this by default!

I will include all these suggestions in the next release. And more hopefully.

Also, I'm wondering if there's a better way to detect this type of situation and abort earlier, without printing hundreds of stacktraces, which can be a bit confusing. Is it intended behavior to keep loading more (what it assumes to be) functions, even though the previous one failed to load? I'm wondering if the first failure should abort the process and not try to keep loading the other functions.

Well, good question. I thought that to make the function as robust as possible, I would couch the require(module) call in a try/catch block. That way if loading any particular module fails, it wont abort the entire deploy process.

Come to think of it, maybe it's better that it does fail and abort, otherwise a function could be missing after a deploy and the developer may not notice (if they don't see the stack trace?).

Let me know what you think.

[hheimbuerger]

This is a bit confusing for me. So, the dash is how firebase denotes a subgroup. So on the module.exports object, which is what the Firebase CLI reads on deployment (and cold start), any particular key can either hold an actual function trigger or another object, which itself holds function triggers. The name of each function depends on the string which is the key for the trigger on the object. The dashes denote the path to the function trigger via the nested objects. For example, { auth: { onCreate: func, onDelete: func2 }} would result in auth-onCreate as the name for func, and auth-onDelete as the name for func2.

Which is why I can't understand how or why this would be different on windows.

I'm pretty sure the nested objects in your case, using today's implementation of BFF on Windows, would be: { 'auth\onCreate': func, 'auth\onDelete': func2 }. That's why no dashes are being added to the function name.

$> firebase functions:shell
i  functions: Loaded functions: http\foo

firebase > http\\foo()
Thrown:
http\\foo()
^^^^^
SyntaxError: Invalid or unexpected token

firebase > this['http\\foo']()
Sent request to function.

So yeah, you can call this function. But it's no fun. 😉

Do you think all of this needs further clarification in the guide and docs?

I believe this is an actual incorrect implementation for Windows. At some point, it's probably modifying directory paths per direct string manipulation, as opposed to using a library that's already OS-aware.

I think there's two ways to deal with this: either a) fix the implementation or b) document that Windows is not supported as a development environment.

The same backslash denotes directories on both windows and Unix based systems, I believe. The only difference is the first part of the path, which starts with C:/... oh. Maybe that's what's breaking it.

No, all path separators are backslashes on Windows, not just the bit right after the drive letter.

Can you do me a favor and run console.log(__filename) on your windows system and paste me the output? There might be a chance that an error is caused when dealing with absolute paths (although I still don't see why...) on a windows system with a drive letter.

C:\dev\bff-scaffold-test\functions\index.js

Also, I'm wondering if there's a better way to detect this type of situation and abort earlier, without printing hundreds of stacktraces, which can be a bit confusing. Is it intended behavior to keep loading more (what it assumes to be) functions, even though the previous one failed to load? I'm wondering if the first failure should abort the process and not try to keep loading the other functions.

Well, good question. I thought that to make the function as robust as possible, I would couch the require(module) call in a try/catch block. That way if loading any particular module fails, it wont abort the entire deploy process.

Come to think of it, maybe it's better that it does fail and abort, otherwise a function could be missing after a deploy and the developer may not notice (if they don't see the stack trace?).

Yeah, that's exactly what I meant. I'm not sure there's a big advantage of seeing the errors from multiple functions at the same time. I do believe the deploy process should be aborted if any particular module fails.

So I would recommend to fail early and fast. Then the developer can focus on one issue, and then run again to identify the next.

[george43g]

:tada: This issue has been resolved in version 3.4.1 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[george43g]

@hheimbuerger you legend - it finally clicked. It's the slash direction. It happens in funcNameFromRelPath. I think I finally understood and fixed this issue.

Can you try one last time and see if it works for you now?

[hheimbuerger]

Updated my test project posted above to 3.4.1.

It still picks up the node_modules directory by default, and (unsuccessfully) tries to load every single module in there as a Firebase function. So diverging from the recommended Firebase project structure by moving your functions into a separate directory and explicitly setting a functionDirectoryPath is still needed.

But when doing that, the new version now generates good function names (in my last example, it would be http.foo, rather than http\foo) on Windows now, good job! 👍