prefer worker package export conditionals if available#93
prefer worker package export conditionals if available#93threepointone merged 8 commits intocloudflare:mainfrom
worker package export conditionals if available#93Conversation
🦋 Changeset detectedLatest commit: 6470f98 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Thanks for the PR! I want to discuss internally what we want it to be called ("cloudflare-worker" feels a bit long to me, but I dunno what a good replacement is just yet.). I'll get back to this when we have some clarity. |
…r package export conditionals if available
|
@threepointone do you think it would make sense to land this now with only support for "worker" condition and then we can add in a more specific condition for "cloudflare-worker" (whatever we decide to name it) in the future? If so, I can update this PR to reflect that change. |
|
Yeah, sounds good to me, let's do that. |
|
Done. Let me know if there is anything else you'd like me to change. |
threepointone
left a comment
There was a problem hiding this comment.
Thanks for this! Left some comments. I'll test this out tomorrow locally and merge it.
cloudflare-worker then worker package export conditionals if availableworker package export conditionals if available
threepointone
left a comment
There was a problem hiding this comment.
This looks fine to me. @Andarist, what do you think? should we add webworker as well?
LGTM too - I've only left some minor suggestions.
I don't think you should include webworker condition yet - cause this has not been defined/promoted anywhere (at least, not just yet, and as far as I've seen, I could have missed something). I believe there were some talks about creating a document that would gather all known "standardized" conditions, one that would live outside of the node/webpack/whatever. I'm not sure what is the status of this though - cc @guybedford |
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
threepointone
left a comment
There was a problem hiding this comment.
Thanks to the both of you! Made the changes, landing this.
This switch/typeof example isn't valid JavaScript AFAICT -- JS doesn't have "type switches" like Go does. NOTE: Unfortunately this example still won't work, because NotFoundError and MethodNotAllowedError are not imported. It doesn't look like kv-asset-handler exports these types currently, so I guess a code change is needed to export them? See #93 (not fully fixed by this PR)
This switch/typeof example isn't valid JavaScript AFAICT -- JS doesn't have "type switches" like Go does. NOTE: Unfortunately this example still won't work, because NotFoundError and MethodNotAllowedError are not imported. It doesn't look like kv-asset-handler exports these types currently, so I guess a code change is needed to export them? See #93 (not fully fixed by this PR)
This switch/typeof example isn't valid JavaScript AFAICT -- JS doesn't have "type switches" like Go does. NOTE: Unfortunately this example still won't work, because NotFoundError and MethodNotAllowedError are not imported. It doesn't look like kv-asset-handler exports these types currently, so I guess a code change is needed to export them? See #93 (not fully fixed by this PR)
* Update config type * Implemented persistState option * Added package link to readme
* Improve detection & usage of deno/bun * Ignore deno.lock file * Fix webpack config * Support cloudflare worker in conditional exports cloudflare/workers-sdk#84 cloudflare/workers-sdk#93 * Fix webpack config * Remove cloudflare conditional exports * Remove publicPath in webpack config * Update webpack.config.js * Finalize PR
Related discussion in #84