Skip to content

fix: warn if the site.entry-point configuration is found during publishing#293

Merged
petebacondarwin merged 1 commit intocloudflare:mainfrom
petebacondarwin:deprecate-sites-entry-point
Jan 25, 2022
Merged

fix: warn if the site.entry-point configuration is found during publishing#293
petebacondarwin merged 1 commit intocloudflare:mainfrom
petebacondarwin:deprecate-sites-entry-point

Conversation

@petebacondarwin
Copy link
Contributor

@petebacondarwin petebacondarwin commented Jan 24, 2022

Is it enough to log a warning? Or should we throw a new DeprecationError()?

Fixes #282

@changeset-bot
Copy link

changeset-bot bot commented Jan 24, 2022

🦋 Changeset detected

Latest commit: 6be9e85

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
wrangler Patch

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 petebacondarwin force-pushed the deprecate-sites-entry-point branch from 0b928c6 to 80833a2 Compare January 24, 2022 22:02
@threepointone
Copy link
Contributor

This should probably have a deprecation error, unless we already throw if we don't find an entry point?

@petebacondarwin
Copy link
Contributor Author

Currently we throw if there is neither a command line script nor a configured build.upload.main property...

Copy link
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one note, but looks good! we can iterate if needed.

@petebacondarwin
Copy link
Contributor Author

I realised that the current flow might be a bit weird for those who have not set up other means for specifying entry-points. They will only receive an error "missing main file". 😢
I'll update this...

@petebacondarwin petebacondarwin force-pushed the deprecate-sites-entry-point branch from 80833a2 to c1e4d12 Compare January 25, 2022 07:35
@petebacondarwin petebacondarwin force-pushed the deprecate-sites-entry-point branch from c1e4d12 to 078523c Compare January 25, 2022 09:27
Copy link
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the tests here deeply, thank you!

@petebacondarwin petebacondarwin force-pushed the deprecate-sites-entry-point branch 2 times, most recently from fe20aed to cf70bd0 Compare January 25, 2022 12:07
…lishing

Also updates the message and adds a test for the error when there is no entry-point specified.

Fixes cloudflare#282
@petebacondarwin petebacondarwin force-pushed the deprecate-sites-entry-point branch from cf70bd0 to 6be9e85 Compare January 25, 2022 12:18
@petebacondarwin petebacondarwin merged commit 71b0fab into cloudflare:main Jan 25, 2022
@petebacondarwin petebacondarwin deleted the deprecate-sites-entry-point branch January 25, 2022 12:32
@github-actions github-actions bot mentioned this pull request Jan 25, 2022
@github-project-automation github-project-automation bot moved this to Untriaged in workers-sdk Nov 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

Deprecate support for sites['entry-point'] configuration: you don't need that if you're explicitly passing the script.

2 participants