Skip to content

Conversation

@Andarist
Copy link
Member

Next.js plans to migrate away from using worker condition because that pulls in React browser builds... so I have been asked to add their dedicated condition.

@Andarist Andarist requested a review from emmatown July 19, 2024 12:58
@changeset-bot
Copy link

changeset-bot bot commented Jul 19, 2024

🦋 Changeset detected

Latest commit: 8fb4d9f

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

This PR includes changesets to release 6 packages
Name Type
@emotion/use-insertion-effect-with-fallbacks Minor
@emotion/primitives Minor
@emotion/styled Minor
@emotion/cache Minor
@emotion/react Minor
@emotion/utils Minor

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

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

"import": "./dist/emotion-cache.development.edge-light.cjs.mjs",
"default": "./dist/emotion-cache.development.edge-light.cjs.js"
},
"worker": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do other libraries not use their browser builds for Service Workers?

Copy link
Member Author

Choose a reason for hiding this comment

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

No idea, there are no great guidelines for this and various tools are not exactly overly consistent and explicit when it comes to differentiating between browser and worker builds.

Webpack 5 docs have always suggested using the worker condition for (web)workers... so that's what we went with here.

But I'm also not sure if I have understood the intention behind your question. Could you elaborate?

Copy link
Member Author

Choose a reason for hiding this comment

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

What we found in the past is that some tools would pick up the browser condition when targeting workers so we had to use some earlier condition (worker) to "shadow" over the browser condition. Ideally, it would just fall through to the default condition and use that

Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering if it makes more sense to use browser builds. But if the browser builds access the DOM, they're not suited for Service Workers. Given that there's little guidance/standardization around this topic we'll probably just have to find out.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ye, so that's the main distinction that we have here - #is-browser could be called #can-use-dom. That's the only thing we care about atm. So we can't use the browser builds because they assume being able to access DOM

@Andarist Andarist merged commit a9f6912 into main Jul 20, 2024
@Andarist Andarist deleted the more-worker-conditions branch July 20, 2024 06:01
@github-actions github-actions bot mentioned this pull request Jul 19, 2024
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.

4 participants