Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

Ensure compatibility with yarn 2 #128

Open
domoritz opened this issue Apr 7, 2021 · 10 comments
Open

Ensure compatibility with yarn 2 #128

domoritz opened this issue Apr 7, 2021 · 10 comments

Comments

@domoritz
Copy link
Contributor

domoritz commented Apr 7, 2021

It would be great if you could add some testing for yarn 2 to prevent issues like #127. Maybe it makes sense to move the repo to yarn 2.

@milesj
Copy link
Collaborator

milesj commented Apr 7, 2021

@domoritz I actually tried upgrading to Yarn 2 before and Beemo was the #1 blocker, so it's definitely on my list. Is that debug PR the only issue with v2 right now?

@domoritz
Copy link
Contributor Author

domoritz commented Apr 7, 2021

Awesome to hear. I'm not sure, to be honest.

The first issue I ran into was that my Beemo config did not expose a beemo command, which is needed for yarn 2 (it does not call bin from a sub-dependency anymore). I fixed it in https://github.com/vega/vega-lite-dev-config/blob/master/package.json#L27.

Another issue I am getting is

> yarn lint

 ERROR

 Failed to resolve a path using the following lookups (in order):
   - @beemo/driver-typescript (NODE_MODULE)
   - beemo-driver-typescript (NODE_MODULE)
  [CMN:PATH_RESOLVE_LOOKUPS]

 STACK TRACE

CommonError: Failed to resolve a path using the following lookups (in order):
  - @beemo/driver-typescript (NODE_MODULE)
  - beemo-driver-typescript (NODE_MODULE)
 [CMN:PATH_RESOLVE_LOOKUPS]
    at PathResolver.resolve (/Users/dominik/Code/vega-embed/.yarn/cache/@boost-common-npm-2.7.2-b09b731933-a36ad3ee2e.zip/node_modules/@boost/common/lib/PathResolver.js:85:13)
    at Loader.load (/Users/dominik/Code/vega-embed/.yarn/cache/@boost-plugin-npm-2.3.3-6a3fe0aca5-6ad401e583.zip/node_modules/@boost/plugin/lib/Loader.js:91:37)
    at Registry.load (/Users/dominik/Code/vega-embed/.yarn/cache/@boost-plugin-npm-2.3.3-6a3fe0aca5-6ad401e583.zip/node_modules/@boost/plugin/lib/Registry.js:139:34)
    at /Users/dominik/Code/vega-embed/.yarn/cache/@boost-plugin-npm-2.3.3-6a3fe0aca5-6ad401e583.zip/node_modules/@boost/plugin/lib/Registry.js:174:140
    at Array.map (<anonymous>)
    at Registry.loadMany (/Users/dominik/Code/vega-embed/.yarn/cache/@boost-plugin-npm-2.3.3-6a3fe0aca5-6ad401e583.zip/node_modules/@boost/plugin/lib/Registry.js:174:111)
    at Tool.bootstrap (/Users/dominik/Code/vega-embed/.yarn/cache/@beemo-core-npm-2.0.0-rc.0-14e61abaec-ff1be644d2.zip/node_modules/@beemo/core/lib/Tool.js:116:31)
    at async /Users/dominik/Code/vega-embed/.yarn/__virtual__/@beemo-cli-virtual-18bbfeb6dc/0/cache/@beemo-cli-npm-2.0.0-rc.0-f11a14682b-a55bf8f48b.zip/node_modules/@beemo/cli/lib/bin.js:30:5
    at async Program.run (/Users/dominik/Code/vega-embed/.yarn/__virtual__/@boost-cli-virtual-182ae6769d/0/cache/@boost-cli-npm-2.10.4-76ceb05708-52cb2a5b21.zip/node_modules/@boost/cli/lib/Program.js:318:9)
    at async Program.runAndExit
(/Users/dominik/Code/vega-embed/.yarn/__virtual__/@boost-cli-virtual-182ae6769d/0/cache/@boost-cli-npm-2.10.4-76ceb05708-52cb2a5b21.zip/node_modules/@boost/cli/lib/Program.js:341:22)

I think that's something in Beemo.

You can see where I am with the migration in vega/vega-embed#664.

@milesj
Copy link
Collaborator

milesj commented Apr 7, 2021

Interesting. The lookup is simply require.resolve(), so I'm curious if Y2 has limitations around that. https://github.com/milesj/boost/blob/master/packages/common/src/PathResolver.ts#L71

Will check, thanks.

@domoritz
Copy link
Contributor Author

domoritz commented Apr 8, 2021

Thanks. I'll put my investigations on ice for now but will try again when something changes here.

@merceyz
Copy link

merceyz commented Apr 8, 2021

The lookup is simply require.resolve(), so I'm curious if Y2 has limitations around that

Not a limitation, it's running require.resolve(foo) which means it will try to resolve @beemo/driver-typescript from itself but it doesn't declare that as a (peer)dependency so access is denied. It would need to either declare it or use the paths option to resolve it from the correct location which is not a PnP only thing, it's the same with node_modules but hoisting usually makes it work. PnP throws an error explaining the undeclared dependency part but since it's in a try catch where the error is ignored it's not surfaced.

@milesj
Copy link
Collaborator

milesj commented Apr 8, 2021

@merceyz Honestly, that's just absolutely mental. All these other libs can't possibly be using paths to solve this.

@merceyz
Copy link

merceyz commented Apr 8, 2021

@milesj
Copy link
Collaborator

milesj commented Apr 13, 2021

Migrated to v2 in node module mode: #130

Will test PnP in a follow-up.

@milesj
Copy link
Collaborator

milesj commented Apr 14, 2021

Tried PnP but will require a handful of changes upstream in Boost, so may be a while.

@milesj
Copy link
Collaborator

milesj commented Jul 12, 2021

Making some progress here: #140

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants