-
Notifications
You must be signed in to change notification settings - Fork 733
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
feat: entry point is not mandatory if --assets
is passed
#1289
Conversation
🦋 Changeset detectedLatest commit: 0709b87 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 |
I was quite surprised at how easy this was 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had an observation, but it shouldn't block this from going live
@@ -35,6 +39,8 @@ export async function getEntry( | |||
? path.resolve(config.site?.["entry-point"]) | |||
: // site.entry-point could be a directory | |||
path.resolve(config.site?.["entry-point"], "index.js"); | |||
} else if (args["experimental-public"]) { | |||
file = path.resolve(__dirname, "../templates/no-op-worker.js"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't immediately obvious that the facade imports this worker - on the surface it looks like we're 404ing all requests 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know, it's spooky lol! I couldn't think of a meaningful test here, open to suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe I'll at least add a comment here for anyone who reads this code in the future
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.developers.workers.dev/runs/2516777968/npm-package-wrangler-1289 You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/1289/npm-package-wrangler-1289 Or you can use npx https://prerelease-registry.developers.workers.dev/runs/2516777968/npm-package-wrangler-1289 dev path/to/script.js |
c17848f
to
7a607ba
Compare
Since we use a facade worker with `--assets`, an entry point is not strictly necessary. This makes a common usecase of "deploy a bunch of static assets" extremely easy to start, as a one liner `npx wrangler dev --assets path/to/folder` (and same with `publish`).
7a607ba
to
0709b87
Compare
--experimental-public
is passed--assets
is passed
Since we use a facade worker with
--assets
, an entry point is not strictly necessary. This makes a common usecase of "deploy a bunch of static assets" extremely easy to start, as a one linernpx wrangler dev --assets path/to/folder
(and same withpublish
).