fix: correctly handle entry-point path when publishing #258
Conversation
🦋 Changeset detectedLatest commit: c940680 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 |
petebacondarwin
force-pushed
the
publish-issue-252
branch
from
064c88f
to
da84c54
Compare
|
I introduced conflicts to your PR in #262 ,sorry! |
petebacondarwin
force-pushed
the
publish-issue-252
branch
from
da84c54
to
af72274
Compare
|
We should land this, but I'm deeply concerned with the ambiguity we've introduced. imo, we should deprecate cc @Electroid |
| } else { | ||
| // If the script name comes from the config, then it is relative to the wrangler.toml file. |
There was a problem hiding this comment.
It took me about two hours to get my head around all these different paths. See also the bit below where we have to resolve the entryPoint paths in the metafile.outputs relative to destination.path!
| @@ -1,7 +1,8 @@ | |||
| import assert from "node:assert"; | |||
| import path from "node:path"; | |||
| import { readFile } from "node:fs/promises"; | |||
| import esbuild from "esbuild"; | |||
| import * as esbuild from "esbuild"; | |||
There was a problem hiding this comment.
By the way, this change was necessary because (for some reason) the default was not being picked up when being run inside Jest...
The `publish` command was failing when the entry-point was specified in the wrangler.toml file and the entry-point imported another file. This was because we were using the `metafile.inputs` to guess the entry-point file path. But the order in which the source-files were added to this object was not well defined, and so we could end up failing to find a match. This fix avoids this by using the fact that the `metadata.outputs` object will only contain one element that has the `entrypoint` property - and then using that as the entry-point path. For runtime safety, we now assert that there cannot be zero or multiple such elements. Fixes cloudflare#252
Watch the files in the wrangler workspace, and run the tests when anything changes: ```sh > npm run test-watch -w wrangler ``` This will also run all the tests in a single process (rather than in parallel shards) and will increase the test-timeout to 50 seconds, which is helpful when debugging.
petebacondarwin
force-pushed
the
publish-issue-252
branch
from
af72274
to
c940680
Compare
The
publishcommand was failing when the entry-point was specified in the wrangler.toml file and the entry-point imported another file.This was because we were using the
metafile.inputsto guess the entry-point file path. But the order in which the source-files were added to this object was not well defined, and so we could end up failing to find a match.This fix avoids this by using the fact that the
metadata.outputsobject will only contain one element that has theentrypointproperty - and then using that as the entry-point path. For runtime safety, we now assert that there cannot be zero or multiple such elements.Fixes #252