Vscode-extension-for-zowe: Migrate from npm to Yarn

Created on 25 Jun 2020  路  13Comments  路  Source: zowe/vscode-extension-for-zowe

Is your feature request related to a problem? Please describe.

This migration is part of the effort for Extender conformance.

Describe the solution you'd like

Describe alternatives you've considered


Additional context

enhancement

Most helpful comment

@zFernand0 yes, that is one key advantage of the monorepo for me as we can now build multiple VS Code extensions and npmjs packages out of the same codebase. That allows us to find breakages immediately by running one test suite and not only after we ship as it happened several times in the last few releases.

All 13 comments

I like this article comparing the different combinations of npm, yarn, lerna: https://doppelmutzi.github.io/monorepo-lerna-yarn-workspaces/

@zFernand0

I started a branch here now using yarn and workspaces packages: https://github.com/zowe/vscode-extension-for-zowe/tree/monorepo-switch

So far I have yarn build, yarn test, yarn package working for the Zowe Explorer package. The vsix file generated is currently not valid as there is a problem with the MS vsce tool and yarn that I can discuss in the Scrum. There is a workaround, but I am exploring a couple of other options.

Thank you @phaumer for creating the branch! I will be happy to learn the details of the issue. I checked out the branch and I am looking at the workaround.

The reason, why webpack does not bundle vscode-nls this config option. @lauren-li why do we exclude all those modules from bundling?

@VitGottwald this is the open vsce bug related to yarn: https://github.com/microsoft/vscode-vsce/issues/300

fyi, I had issues with yarn and vsce as well. In the github actions workflows, I needed npm to install.

Otherwise, I used yarn commands in the github actions as I could for consistency.

@VitGottwald If I understand correctly, vscode-nls was originally excluded from being webpacked because it needed to use the non-packed vscode-nls code at runtime (hence why it also needs to be included as !node_modules/vscode-nls/** in .vscodeignore).

However, I checked again after your question, and apparently CPPTools (which I used as an example when implementing webpack in Zowe Explorer) removed vscode-nls from their webpack externals config a while ago, and it seems to still be working for them. See their commit: https://github.com/microsoft/vscode-cpptools/pull/4884
@phaumer will try out a build that includes vscode-nls to see if this will also work for us. Thanks Peter!

Ok, it seems to work now with the latest vscode-nls to have it webpacked. I added a commit that fixes the issue.

Thank you @phaumer and @lauren-li . I added another commit to simplify the webpack config and command line a bit.

Thank you @phaumer and @VitGottwald!

Just a note: Vit, it looks like the commit you added removes the --vscode-nls option (which could be omitted in development build scripts to build more quickly without vscode-nls when we are working on non-nls-related features). However, I think this should be fine, as we always had --vscode-nls turned on in all our build scripts that use webpack anyways (i.e. no losses compared with what is already in the master branch). But just in case we do ever want to bring that --vscode-nls option back, here is the reference to the commit: https://github.com/zowe/vscode-extension-for-zowe/commit/592ea5554dbd03155c5fcc6ffe00c0566b705bc8

I extracted the api pieces into its own package and fixed tests. I also added prettier as all the files were moved into a new folder loosing the file history now is the perfect time to add it. I relaxed some of the tslint rules for that, but the goal is to replace this with eslint anyway.

Next todos:

  • There one test where I was not able to make the mock work. Any ideas anyone? packages/zowe-explorer/__tests__/__unit__/utils.unit.test.ts, Line 100?
  • Currently the strings used in the files extracted into the api package are not loaded and there is an error about that at startup. Ideally the API should be free of strings or we need to explore how npm packages can contribute strings to the extension
  • We should move some api related tests into the package, but I would wait for now as I want to simplify the API, e.g. split in the Profiles in a model (without strings) and a view.
  • Add FTP extension next to test the API with both apps.
  • I need help with the automation scripts

Hey @phaumer
I might be able to help with some of the automation scripts (since I have #907 assigned to me) :yum:

I do have a question regarding one specific todo:

- Add FTP extension next to test the API with both apps.

Does this mean that we plan to "combine" pieces of the FTP extension in this monorepo (zowe/vscode-extendion-for-zowe)?
Please correct me if I got it wrong. I may be misunderstanding that specific item.

@zFernand0 yes, that is one key advantage of the monorepo for me as we can now build multiple VS Code extensions and npmjs packages out of the same codebase. That allows us to find breakages immediately by running one test suite and not only after we ship as it happened several times in the last few releases.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

katelynienaber picture katelynienaber  路  3Comments

jelaplan picture jelaplan  路  3Comments

zdmullen picture zdmullen  路  3Comments

AHumanFromCA picture AHumanFromCA  路  3Comments

fritzc1 picture fritzc1  路  3Comments