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

Make functions build path selection smarter: build path changes based on imports #2625

Closed
bsmith418 opened this issue Sep 20, 2020 · 7 comments

Comments

@bsmith418
Copy link

If you import from your Angular project into index.js:

import {ExampleModel} from '../../src/app/example.model';

The build directory changes from
"lib/index.js"
to
"lib/functions/src/index.js"

Then when you try to "firebase deploy", firebase-tools will either break or deploy your old build at lib/index.js. If the latter happens, you'll have no errors and a great deal of confusion trying to figure out why your functions aren't updating.

@samtstern
Copy link
Contributor

@bsmith418 sorry I don't quite understand what's going on here. firebase-tools doesn't really care where your code is, you just need to tell us in firebase.json or we assume the defaults which are:

  • All code is in functions directory
  • The main file with the functions exports is functions/index.js

These pages may be useful to you:

@bsmith418
Copy link
Author

bsmith418 commented Sep 21, 2020

It is disrespectful to close the issue without asking clarifying questions. I took time out of my day to detail a problem, only to have what I was saying misunderstood and condescendingly dismissed. "Developer Relations" indeed.

That aside, here is another go at explaining what is happening.

Expected behavior:
When you build and firebase deploy, your Functions get deployed.

Actual behavior:
When you build and firebase deploy, your Functions may or may not get deployed. firebase deploy might mysteriously deploy an old version of index.js.

Case study:

A developer make a simple index.js. She builds firebase deploys, and it uploads her function to Firebase Functions. No problem.

She decides to include a model.ts from her app's source code. Accordingly, she imports /exampleApp/src/app/example.model.ts into /exampleApp/functions/src/index.js.

She builds and firebase deploy's. No errors, so everything seems to be fine.

However, when she checks on her Function, she finds that the old version is still running. There were no errors, so she now has to sort through all of these possible problems:

  • Does Firebase Functions require more time to propagate the latest version?
  • Was there a hidden error in her code, which is making Firebase Functions refuse to switch to the new version?
  • Is ng build failing somehow?
  • Is the version checking on firebase deploy failing somehow?
  • Does she need to poke around in the Firebase Console to switch Functions to a new version?
  • Does she need to update firebase tools?
  • Is the Typescript implementation of Firebase Functions buggy? Should she switch to Javascript?
  • ... and onward.

The obscure truth is that an import into index.js might change the build directory, and the new build directory will have to be accounted for in package.json. This change in build directory happens under the hood, without notification or documentation. And thus, firebase deploy was uploading her old version in the old build directory, and not the new index.js in the new, esoterically generated build directory, and all of the above more-probable-seeming hypotheses were false.

In summary, most developers expect ng build followed by firebase deploy to have predictable behavior. The actual behavior is far from predictable: it depends on your imports in index.js.

The likely solution to all of this is to throw a warning in firebase tools when index.js includes an import from an ancestor or sibling of the /functions/ directory.

@samtstern samtstern reopened this Sep 25, 2020
@samtstern
Copy link
Contributor

@bsmith418 I apologize if you felt disrespected, that wasn't my intention. The reason I closed the issue so quickly was because it did not follow the issue template and from my initial reading it was out of scope for this issue tracker, as it seems like a complication with building Angular apps. Without detailed reproduction steps, I often can't do anything.

That said I still think this is not an issue with the Firebase CLI, it sounds like more of something to do with how the ng tool decides to build your code. Specifically it should at least be cleaning up index.js in the lib folder if it's out of date. Then the firebase tool would try and find your code and fail to find it. We don't really have any opportunity to example the imports in your functions/index.js, we require() your code which lets us examine the exported function handlers.

Does that make sense?

@bsmith418
Copy link
Author

@samtstern No worries. I would imagine there is some degree of coordination between the Firebase and Angular teams? Changing the build process indeed seems cleaner. The problem scope / failure point is in Firebase, but the elegant solution is in Angular. It seems solvable on either end, and devs only care about consistent or transparent behavior.

@samtstern
Copy link
Contributor

@bsmith418 hmmm I see where you're coming from but I don't really agree that Firebase has failed here. The firebase.json/package.json config says the main file is in lib/index.js. If that file exists, we deploy it. The fact that another build tool has moved the file or left junk around on the system is not something we can police.

Have you reported this issue to the Angular team?

@samtstern
Copy link
Contributor

@bsmith418 so funny enough this exact issue happened to me last night while working on a personal project. My project doesn't use Angular, but it does use TypeScript and I had some code in functions that imported from ../shared/utils.ts and it moved my functions/lib/index.js file to functions/lib/functions/src/index.js!

So first of all thank you, since I immediately recognized the issue as the same one you had and didn't have to do any head scratching. Second of all I think this is something we could fix through tsconfig but I am not sure.

@samtstern
Copy link
Contributor

Ok so apparently this is a well-worn issue (ex: microsoft/TypeScript#16683) and the official answer is:
https://github.com/Microsoft/TypeScript/wiki/FAQ#why-does---outdir-moves-output-after-adding-a-new-file

There are some discussions here about how to do a "clean build" of a TypeScript project:
microsoft/TypeScript#16057

You could always do a simple rimraf ./lib in your predeploy script but based on the conversations above that's not universally safe to do so I don't think we can add it to the firebase init templates.

Unfortunately my conclusion is that this is a TS quirk that is definitely painful but there's nothing Firebase can do here. When we see index.js we have no way of knowing that it's "out of date" or that the real main file has now moved. We don't/can't analyze the TS source so we can't even warn that this may happen. All we could do is detect if the index.js file is missing, but there's no universally safe way to do a clean TS build so we can't even guarantee that.

If you can think of a better answer please let me know and I will re-open this! But for now I'm going to close it as something we can't feasibly improve. I hope one day TS adds a simple tsc --clean option which we would gladly adopt.

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

No branches or pull requests

2 participants