Skip to content
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

fix: resolve npm modules correctly #656

Merged
merged 1 commit into from
Mar 21, 2022
Merged

fix: resolve npm modules correctly #656

merged 1 commit into from
Mar 21, 2022

Conversation

threepointone
Copy link
Contributor

When implementing legacy module specifiers, we didn't throughly test the interaction when there weren't any other files next to the entry worker, and importing npm modules. It would create a Regex that matched every import, and fail because a file of that name wasn't present in the source directory. This fix constructs a better regex, applies it only when there are more files next to the worker, and increases test coverage for that scenario.

Fixes #655


(ignore whitespace when reviewing this)

When implementing legacy module specifiers, we didn't throughly test the interaction when there weren't any other files next to the entry worker, and importing npm modules. It would create a Regex that matched _every_ import, and fail because a file of that name wasn't present in the source directory. This fix constructs a better regex, applies it only when there are more files next to the worker, and increases test coverage for that scenario.

Fixes #655
@changeset-bot
Copy link

changeset-bot bot commented Mar 21, 2022

🦋 Changeset detected

Latest commit: 4247ea5

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

@github-actions
Copy link
Contributor

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/2016722948/npm-package-wrangler-656

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/656/npm-package-wrangler-656

Or you can use npx with this latest build directly:

npx https://prerelease-registry.developers.workers.dev/runs/2016722948/npm-package-wrangler-656 dev path/to/script.js

@threepointone threepointone merged commit aeb0fe0 into main Mar 21, 2022
@threepointone threepointone deleted the fix-655 branch March 21, 2022 14:58
@github-actions github-actions bot mentioned this pull request Mar 21, 2022
mrbbot added a commit that referenced this pull request Oct 31, 2023
This is a temporary fix to allow Workers Sites tests cherrypicked
from #656 to pass. Listing keys in Workers Sites `__STATIC_CONTENT`
namespace was previously broken, due to a type error in the inline
namespace worker script. This change also means we can avoid the
`Function#toString()`ing to "bundle" code in inline worker scripts.
mrbbot added a commit that referenced this pull request Oct 31, 2023
mrbbot added a commit that referenced this pull request Oct 31, 2023
With #656, the Queues dispatcher is now implemented as part of the
broker Durable Object. We no longer send message batches directly
from Node.js, so can remove queue handling from the entry worker.

Note the magic proxy enqueues messages through queue producer
bindings like regular workers, so never used this endpoint directly.
mrbbot added a commit that referenced this pull request Nov 1, 2023
This is a temporary fix to allow Workers Sites tests cherrypicked
from #656 to pass. Listing keys in Workers Sites `__STATIC_CONTENT`
namespace was previously broken, due to a type error in the inline
namespace worker script. This change also means we can avoid the
`Function#toString()`ing to "bundle" code in inline worker scripts.
mrbbot added a commit that referenced this pull request Nov 1, 2023
mrbbot added a commit that referenced this pull request Nov 1, 2023
With #656, the Queues dispatcher is now implemented as part of the
broker Durable Object. We no longer send message batches directly
from Node.js, so can remove queue handling from the entry worker.

Note the magic proxy enqueues messages through queue producer
bindings like regular workers, so never used this endpoint directly.
mrbbot added a commit that referenced this pull request Nov 1, 2023
This is a temporary fix to allow Workers Sites tests cherrypicked
from #656 to pass. Listing keys in Workers Sites `__STATIC_CONTENT`
namespace was previously broken, due to a type error in the inline
namespace worker script. This change also means we can avoid the
`Function#toString()`ing to "bundle" code in inline worker scripts.
mrbbot added a commit that referenced this pull request Nov 1, 2023
mrbbot added a commit that referenced this pull request Nov 1, 2023
With #656, the Queues dispatcher is now implemented as part of the
broker Durable Object. We no longer send message batches directly
from Node.js, so can remove queue handling from the entry worker.

Note the magic proxy enqueues messages through queue producer
bindings like regular workers, so never used this endpoint directly.
mrbbot added a commit that referenced this pull request Nov 1, 2023
This is a temporary fix to allow Workers Sites tests cherrypicked
from #656 to pass. Listing keys in Workers Sites `__STATIC_CONTENT`
namespace was previously broken, due to a type error in the inline
namespace worker script. This change also means we can avoid the
`Function#toString()`ing to "bundle" code in inline worker scripts.
mrbbot added a commit that referenced this pull request Nov 1, 2023
mrbbot added a commit that referenced this pull request Nov 1, 2023
With #656, the Queues dispatcher is now implemented as part of the
broker Durable Object. We no longer send message batches directly
from Node.js, so can remove queue handling from the entry worker.

Note the magic proxy enqueues messages through queue producer
bindings like regular workers, so never used this endpoint directly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 BUG: Wrangler fails to compile worker when using modules that are compatible with workers (via NPM)
2 participants