build icon indicating copy to clipboard operation
build copied to clipboard

Add `constants.EDGE_HANDLERS_DIST`

Open ehmicky opened this issue 5 years ago • 10 comments

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 avatar Sep 17 '20 19:09 ehmicky

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

shortdiv avatar Sep 17 '20 20:09 shortdiv

Done at https://github.com/netlify/build/pull/1830

ehmicky avatar Sep 18 '20 20:09 ehmicky

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 avatar Sep 18 '20 20:09 mraerino

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

mraerino avatar Sep 18 '20 20:09 mraerino

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?

ehmicky avatar Sep 18 '20 20:09 ehmicky

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

mraerino avatar Sep 18 '20 20:09 mraerino

  • 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.

erezrokah avatar Sep 20 '20 10:09 erezrokah

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 avatar Sep 21 '20 11:09 ehmicky

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.

ehmicky avatar Sep 24 '20 10:09 ehmicky

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

mraerino avatar Sep 24 '20 12:09 mraerino

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[bot] avatar Oct 11 '22 14:10 github-actions[bot]

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

github-actions[bot] avatar Oct 25 '22 14:10 github-actions[bot]