Skip to content

feat: extract runtime-handler and lazyLoading #252

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

Merged
merged 8 commits into from
May 20, 2021

Conversation

dkundel
Copy link
Contributor

@dkundel dkundel commented Apr 30, 2021

This is still in progress. I still have to verify the functionality and port/add tests. As part of this change the new runtime-handler is now using the twilio package that person has installed.

In the past we were relying on the resolution of Node.js to pick up the right one but it might pick up the wrong one.

fixes: #250

Contributing to Twilio

All third-party contributors acknowledge that any contributions they provide will be made under the same open-source license that the open-source project is provided under.

  • I acknowledge that all my contributions will be made under the project's license.

@dkundel
Copy link
Contributor Author

dkundel commented Apr 30, 2021

This PR will also officially drop Node 10 support

configureWatcher(config, server);
return server.getApp();
} catch (err) {
debug(
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to catch this if the version of twilio-run depends on runtime-handler? What is the situation in which this version of twilio-run would not have access to a LocalDevelopmentServer from this package, regardless of which version it is?

Copy link
Contributor

Choose a reason for hiding this comment

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

And consequently, do we need to keep all the server code in twilio-run as well as in runtime-handler?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it doesn't actually depend on runtime-handler. It's a devDependency for the type but the require comes from the customers project not bundled from twilio-run. We could do both but I feel like that might introduce more edge cases than just ripping the bandaid off in the next major release and dropping all the code.

@dkundel
Copy link
Contributor Author

dkundel commented Apr 30, 2021

The PR should be ready for review but shouldn't be merged yet which is why it's in progress

@dkundel dkundel changed the base branch from main to features/runtime-handler May 20, 2021 23:35
@dkundel dkundel marked this pull request as ready for review May 20, 2021 23:38
@dkundel dkundel changed the title feat: extract runtime-handler feat: extract runtime-handler and lazyLoading May 20, 2021
@dkundel dkundel merged commit 4b11e69 into features/runtime-handler May 20, 2021
@dkundel dkundel deleted the dkundel/fix-250 branch May 20, 2021 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate runtime out of twilio-run
2 participants