Skip to content

Add constants.EDGE_HANDLERS_DIST #1822

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

Closed
ehmicky opened this issue Sep 17, 2020 · 12 comments
Closed

Add constants.EDGE_HANDLERS_DIST #1822

ehmicky opened this issue Sep 17, 2020 · 12 comments
Labels
project:edge-handlers-support stale type: feature code contributing to the implementation of a feature and/or user facing functionality

Comments

@ehmicky
Copy link
Contributor

ehmicky commented Sep 17, 2020

Some plugins have previously wanted to know the output directory where Functions are bundled. We provide with a non-configurable constants.FUNCTIONS_DIST for this purpose.

We might have the same need for Edge handlers. At the moment, the output directory is hard-coded as .netlify/edge-handlers. However, we probably should follow the same pattern and expose it as a non-configurable constants.EDGE_HANDLERS_DIST instead so Build plugins can use this value.

@mraerino @calavera @shortdiv What do you think?

@ehmicky ehmicky added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Sep 17, 2020
@shortdiv
Copy link
Contributor

I really like this idea! I think considering that users will be able to configure the edge handlers directory and user edge handler plugins will build to that directory, we'd want this capability

@ehmicky
Copy link
Contributor Author

ehmicky commented Sep 18, 2020

Done at #1830

@mraerino
Copy link
Contributor

mraerino commented Sep 18, 2020

somehow i missed this specific issue. so sorry for being late to the party.

comment from the PR:

i have a strong opinion that this constant should be the choice of the plugin, not of netlify-build.
we are using that constant in other contexts like

  • uploading handlers from CLI
  • running handlers locally via traffic mesh

it doesn't make sense to move setting this to netlify-build. it's not something we consider good to use by anyone from the outside right now

@mraerino
Copy link
Contributor

why is this different from functions?

we have a specific bundling format that is not openly documented and relies on a bunch of implementation details in our runtime. third parties won't be able to produce meaningful output into this directory

@ehmicky
Copy link
Contributor Author

ehmicky commented Sep 18, 2020

Yes, this makes sense, especially your last argument. Thanks for chiming in before we ship this.

My only concern would be if some Plugins started to reverse engineer .netlify/edge-handlers bundles and their contents. They would then hard-code the directory location. However, this can arguably be considered a hack, and we could prevent it during plugins code review.
Based on this, I agree we should close this issue. What do you think @shortdiv?

@mraerino
Copy link
Contributor

yeah, we don't have to support hacks. that's the good thing about not providing this path openly

@erezrokah
Copy link
Contributor

  • uploading handlers from CLI

I would actually prefer if this was exposed in some way and not hardcoded in the CLI.
Also, @ehmicky do we need to resolve the path based on the site root here:
https://github.com/netlify/netlify-plugin-edge-handlers/blob/a9f8b0b11a1a476876966310a04ecc7acb051701/src/consts.js#L3
?

Using the plugin as a direct dependency in the CLI feels odd and I'm not sure it's worth separating that logic into a separate package just for the sake of exporting some consts.

@ehmicky
Copy link
Contributor Author

ehmicky commented Sep 21, 2020

I personally don't have any strong preference either way, so will let @mraerino and @shortdiv chime in.

Also, @ehmicky do we need to resolve the path based on the site root here: netlify/netlify-plugin-edge-handlers@a9f8b0b/src/consts.js#L3?

Yes, this is relative to the build directory (called this.api.site.root in the CLI).

@ehmicky
Copy link
Contributor Author

ehmicky commented Sep 24, 2020

While writing the documentation, the following potential use case was brought up: users might want to debug Edge handlers bundling by viewing the output bundles. This would require knowledge and documentation of .netlify/edge-handlers/. I don't know whether this is a good idea, but thought this belonged to this discussion.

@mraerino
Copy link
Contributor

mraerino commented Sep 24, 2020

we don't need it right now, but we can keep it in the backlog for when people run into issues with the process

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had activity in 1 year. It will be closed in 14 days if no further activity occurs. Thanks!

@github-actions github-actions bot added the stale label Oct 11, 2022
@github-actions
Copy link
Contributor

This issue was closed because it had no activity for over 1 year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project:edge-handlers-support stale type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants