-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add Dart runtime delegate #3
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
base: master
Are you sure you want to change the base?
Conversation
| // For Dart, include the function name in the path so the server can route | ||
| // For other runtimes, use / as they use FUNCTION_TARGET env var | ||
| const isDart = runtime?.startsWith("dart"); | ||
| const path = isDart ? `/${trigger.entryPoint}` : `/`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about this
| private staticBackends: EmulatableBackend[] = []; | ||
| private dynamicBackends: EmulatableBackend[] = []; | ||
| private watchers: chokidar.FSWatcher[] = []; | ||
| private buildRunnerProcesses: Map<string, ChildProcess> = new Map(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, feels like we are polluting something generic
|
|
||
| // For Dart, include the function name in the path so the server can route | ||
| // For other runtimes, use / as they use FUNCTION_TARGET env var | ||
| const isDart = runtime?.startsWith("dart"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels like this should be easier to check? E.g. for each runtime add a runtime type enum/const, so we can do:
runtime.type === RUNTIMES.DART
Or something... maybe out of scope of the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or instanceof DartDelegate
| dependencies: | ||
| firebase_functions: | ||
| path: ../ | ||
| shelf: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Remove once we wrap shelf types
| /.+?[\\\/]venv[\\\/].+?/, // Ignore site-packages in venv | ||
| ...(backend.ignore?.map((i) => `**/${i}`) ?? []), | ||
| ], | ||
| ignored: isDart |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, wondering if it's not polluting too much, maybe we should try to integrate with the
async build(): Promise<void> {
Description
Scenarios Tested
Sample Commands