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

Migrate from npm to Yarn #905

Closed
zdmullen opened this issue Jun 25, 2020 · 13 comments
Closed

Migrate from npm to Yarn #905

zdmullen opened this issue Jun 25, 2020 · 13 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@zdmullen
Copy link
Contributor

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

@phaumer
Copy link
Member

phaumer commented Jul 21, 2020

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

@zFernand0

@crawr crawr modified the milestones: 1.8 Release, 1.9 Release Jul 31, 2020
@phaumer phaumer self-assigned this Aug 10, 2020
@phaumer
Copy link
Member

phaumer commented Aug 11, 2020

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.

@VitGottwald
Copy link
Contributor

VitGottwald commented Aug 11, 2020

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.

@VitGottwald
Copy link
Contributor

VitGottwald commented Aug 11, 2020

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

@phaumer
Copy link
Member

phaumer commented Aug 11, 2020

@VitGottwald this is the open vsce bug related to yarn: microsoft/vscode-vsce#300

@dkelosky
Copy link
Contributor

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.

@lauren-li
Copy link
Contributor

@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: microsoft/vscode-cpptools#4884
@phaumer will try out a build that includes vscode-nls to see if this will also work for us. Thanks Peter!

@phaumer
Copy link
Member

phaumer commented Aug 11, 2020

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

@VitGottwald
Copy link
Contributor

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

@lauren-li
Copy link
Contributor

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: 592ea55

@phaumer
Copy link
Member

phaumer commented Aug 13, 2020

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

@zFernand0
Copy link
Member

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

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.

@phaumer
Copy link
Member

phaumer commented Aug 17, 2020

@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.

zFernand0 pushed a commit that referenced this issue Nov 4, 2020
* Created packages. Fixed building.

Signed-off-by: Peter Haumer <[email protected]>

* VSIX patch script. Fixed launches.

Signed-off-by: Peter Haumer <[email protected]>

* Added vscode-nls to webpack

Signed-off-by: Peter Haumer <[email protected]>

* Simplified webpack config

* Move eslint plugin devDependency

* Switch github actions to yarn

* Switch theia github action to yarn

* Use yarn for installs and script running

* Extracted explorer-api and refactored

Signed-off-by: Peter Haumer <[email protected]>

* Added prettier

Signed-off-by: Peter Haumer <[email protected]>

* Renamed pub folder to dist

Signed-off-by: Peter Haumer <[email protected]>

* add/update scripts for pre/posttest:integration

Signed-off-by: Billie Simmons <[email protected]>

* Added ftp package files

Signed-off-by: Peter Haumer <[email protected]>

* Enabling webpack for FTP

Signed-off-by: Peter Haumer <[email protected]>

* Fixed eslint for ftp

Signed-off-by: Peter Haumer <[email protected]>

* Added FTP launch

Signed-off-by: Peter Haumer <[email protected]>

* Add DCO signoff for past commits

* Pretty 120 zowe explorer

Signed-off-by: Peter Haumer <[email protected]>

* Pretty 120 api

Signed-off-by: Peter Haumer <[email protected]>

* Pretty 120 ftp

Signed-off-by: Peter Haumer <[email protected]>

* Prettier top level

Signed-off-by: Peter Haumer <[email protected]>

* Merging 1.10 master branch

Signed-off-by: Peter Haumer <[email protected]>

* Prettier 120/4

Signed-off-by: Peter Haumer <[email protected]>

* Removed test profile

Signed-off-by: Peter Haumer <[email protected]>

* Updated gitignore

Signed-off-by: Peter Haumer <[email protected]>

* Add missing messages

Signed-off-by: zFernand0 <[email protected]>

* yarn package fixes

Signed-off-by: Peter Haumer <[email protected]>

* Move testProfileData.ts file creation to a script in zowe-explorer.
Remove it from workflow.
Trigger workflow only on changes to the zowe-explorer package

Signed-off-by: zFernand0 <[email protected]>

* Quick fix for uploading test results

Signed-off-by: zFernand0 <[email protected]>

* yarn and vsce devDeps

Signed-off-by: Peter Haumer <[email protected]>

* Profiles split

Signed-off-by: Peter Haumer <[email protected]>

* add yarnrc file

Signed-off-by: zFernand0 <[email protected]>

* Fixed imports

Signed-off-by: Peter Haumer <[email protected]>

* String updates

Signed-off-by: Peter Haumer <[email protected]>

* Moved another UI method

Signed-off-by: Peter Haumer <[email protected]>

* Update filepaths for monorepo

Signed-off-by: Lauren Li <[email protected]>

Co-authored-by: Vit Gottwald <[email protected]>
Co-authored-by: Billie Simmons <[email protected]>
Co-authored-by: zFernand0 <[email protected]>
Co-authored-by: Lauren Li <[email protected]>

Signed-off-by: zFernand0 <[email protected]>
zFernand0 pushed a commit that referenced this issue Nov 19, 2020
* Created packages. Fixed building.

Signed-off-by: Peter Haumer <[email protected]>

* VSIX patch script. Fixed launches.

Signed-off-by: Peter Haumer <[email protected]>

* Added vscode-nls to webpack

Signed-off-by: Peter Haumer <[email protected]>

* Simplified webpack config

* Move eslint plugin devDependency

* Switch github actions to yarn

* Switch theia github action to yarn

* Use yarn for installs and script running

* Extracted explorer-api and refactored

Signed-off-by: Peter Haumer <[email protected]>

* Added prettier

Signed-off-by: Peter Haumer <[email protected]>

* Renamed pub folder to dist

Signed-off-by: Peter Haumer <[email protected]>

* add/update scripts for pre/posttest:integration

Signed-off-by: Billie Simmons <[email protected]>

* Added ftp package files

Signed-off-by: Peter Haumer <[email protected]>

* Enabling webpack for FTP

Signed-off-by: Peter Haumer <[email protected]>

* Fixed eslint for ftp

Signed-off-by: Peter Haumer <[email protected]>

* Added FTP launch

Signed-off-by: Peter Haumer <[email protected]>

* Add DCO signoff for past commits

* Pretty 120 zowe explorer

Signed-off-by: Peter Haumer <[email protected]>

* Pretty 120 api

Signed-off-by: Peter Haumer <[email protected]>

* Pretty 120 ftp

Signed-off-by: Peter Haumer <[email protected]>

* Prettier top level

Signed-off-by: Peter Haumer <[email protected]>

* Merging 1.10 master branch

Signed-off-by: Peter Haumer <[email protected]>

* Prettier 120/4

Signed-off-by: Peter Haumer <[email protected]>

* Removed test profile

Signed-off-by: Peter Haumer <[email protected]>

* Updated gitignore

Signed-off-by: Peter Haumer <[email protected]>

* Add missing messages

Signed-off-by: zFernand0 <[email protected]>

* yarn package fixes

Signed-off-by: Peter Haumer <[email protected]>

* Move testProfileData.ts file creation to a script in zowe-explorer.
Remove it from workflow.
Trigger workflow only on changes to the zowe-explorer package

Signed-off-by: zFernand0 <[email protected]>

* Quick fix for uploading test results

Signed-off-by: zFernand0 <[email protected]>

* yarn and vsce devDeps

Signed-off-by: Peter Haumer <[email protected]>

* Profiles split

Signed-off-by: Peter Haumer <[email protected]>

* add yarnrc file

Signed-off-by: zFernand0 <[email protected]>

* Fixed imports

Signed-off-by: Peter Haumer <[email protected]>

* String updates

Signed-off-by: Peter Haumer <[email protected]>

* Moved another UI method

Signed-off-by: Peter Haumer <[email protected]>

* Update filepaths for monorepo

Signed-off-by: Lauren Li <[email protected]>

Co-authored-by: Vit Gottwald <[email protected]>
Co-authored-by: Billie Simmons <[email protected]>
Co-authored-by: zFernand0 <[email protected]>
Co-authored-by: Lauren Li <[email protected]>

Signed-off-by: zFernand0 <[email protected]>
@zFernand0 zFernand0 modified the milestones: 1.9 Release, 1.10 Release Nov 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants