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

Worklet support #5837

Closed
wants to merge 2 commits into from
Closed

Worklet support #5837

wants to merge 2 commits into from

Conversation

ProLoser
Copy link

↪️ Pull Request

Support for worklets.

💻 Examples

import url from 'worklet:./my-worklet.js'

🚨 Test instructions

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@height
Copy link

height bot commented Feb 14, 2021

Link Height tasks by mentioning a task ID in the pull request title or description, commit messages, or comments.

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.


export default (new Transformer({
transform({asset}) {
asset.symbols.clear();
Copy link
Member

Choose a reason for hiding this comment

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

Can you give us some context about why this plugin is necessary?

Copy link
Author

Choose a reason for hiding this comment

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

@mischnic perhaps you could speak to this?

Copy link
Author

Choose a reason for hiding this comment

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

I'm not really sure how this.#value.symbols is used, the worklet or worker will likely not be able to share dependencies with the main javascript thread (for now) so I'm curious if this is an attempt to prevent the asset from having additional globals or other dependencies incorrectly bundled into the asset during packaging?

Copy link
Member

Choose a reason for hiding this comment

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

This is purely to workaround the foo.js does not export 'default' error when doing import x from "worker:./foo.js", has nothing to do with shared bundles.

An alternative is special-casing asset.isIsolated in the symbol propagation logic itself.

@ProLoser
Copy link
Author

ProLoser commented Feb 18, 2021

I wouldn't need this branch merged if I knew how to run parcel commands without having it mess with the dependencies, is there a flag for that?

@mischnic
Copy link
Member

  1. (Turn your project into a monorepo)
  2. Add the newly added transformers to your project as packages
  3. Add them to the parcelrc

@ProLoser
Copy link
Author

ProLoser commented Feb 18, 2021

  1. Already a monorepo
  2. I copied transformers/clear, transformers/worker, transformers/worklet
  3. Add them to the .parcelrc

Cleared node modules and reinstalled, I'm getting lots of packaging headaches. I've also tried making the parcel repo a submodule (on the worker-url branch) and adding "parcel/packages/*/*" to my package.json#workspaces[] but it's nothing but dependency headaches and version mismatches.

image

@mischnic
Copy link
Member

I haven't had any problems so far with adding a Parcel plugin to a monorepo, for example here: https://github.com/mischnic/parcel-resolver-root. example would be your project

@ProLoser
Copy link
Author

Okay I got it working and it was really easy in the end:

cd myProject; git submodule add -b worker-url https://github.com/parcel-bundler/parcel.git parcel

and then I updated my monorepo config:

// myProject/package.json
{
	"workspaces": [
		  "packages/*", // mine
		  "parcel/packages/*/*" // yours
	],
	"dependencies": {
		"parcel": "2.0.0-beta.1"
	}
}

The problem I had was I was previously using nightly releases, but the packages from worker-url are set to 2.0.0-beta.1 and I really don't want to keep manually updating those with every nightly.

So instead, I decided it's easier to treat the entire parcel dependency tree as a submodule that I can checkout whatever version I want. To get the dependency to work correctly, I had to ensure that my root package.json was pointing to the same version as the one specified in the submodule, or else the submodule would not be considered a recent enough release to satisfy the dependency (even though the source code itself IS in fact newer).

I would recommend these instructions for simplicity and minimal dependency headache in the future ^

@ProLoser
Copy link
Author

ProLoser commented Feb 18, 2021

I should mention that the way you guys are setting up your dependencies on @babel/core is causing multiple copies of babel to be resolved in different packages.

I would recommend peaking at your yarn.lock and running yarn why @babel/core to see how many versions are being resolved. I'm seeing @babel/core@7.12.9, @babel/core@7.12.16 and @babel/core@7.12.17. Although deleting my yarn.lock and reinstalling deduplicated them, but either way I was concerned this may cause 2 separate javascript memory runtimes in parcel, or you know like loading the same file in 2 locations.

@ProLoser
Copy link
Author

ProLoser commented Feb 18, 2021

I'm still seeing this when I do a build instead of serve
image

@ProLoser
Copy link
Author

image

I've seen this typically when running built files. The build succeeds but running the code does not. Serving the files succeeds and running succeeds.

@mischnic
Copy link
Member

Sounds like #5816

@ProLoser
Copy link
Author

Yup that looks like what I'm experiencing and it looks like it was fixed after this branch was created

@ProLoser
Copy link
Author

I rebased this branch on top of the latest v2 locally and tried building again but it's still showing up for me :/ any tips what I should be looking at?

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.

3 participants