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

Revamp build process and update to 1.96.3 #555

Merged
merged 26 commits into from
Jan 21, 2025
Merged

Conversation

CGNonofr
Copy link
Contributor

@CGNonofr CGNonofr commented Jan 20, 2025

I'm aware it's a HUGE PR, sorry for that, I don't really expect you to fully understand what has been done about the build process as it would be really time consuming.

It's tried to fix all the issues I've noticed about the build process and decided to fully rework it on healthier bases.

What was improved:

  • The packages dependencies should now be properly set on all the subpackages
  • Each vscode module now has its .d.ts next to it, which allows to import any vscode file from the final project with the types
  • It's faster (there is now only 2 rounds of rollup instead of 3, and no separate types bundling)

Regarding the type, I had to make a choice, because the import graph of types and the one from the actual ESM are different (for instance, if only a type is imported from another module, the import is removed from the generated ESM).

I've decided that only external dependencies imported from actual ESM files are declared in the package dependencies. External dependencies only imported from d.ts files are put in the peerDependencies as optional (using peerDependenciesMeta). Any though about it?

Also, I've upgraded VSCode in the same MR so a new major release is triggered in case something was broken in the process.

And whilte I'm at it, I've updated every single dependencies, including eslint which I've switch to the default recommended configuration for now

@CGNonofr
Copy link
Contributor Author

@kaisalmen do you have an idea about the build error? I've tried many things without any differences 🤔

No issue on my machine of course

@kaisalmen
Copy link
Collaborator

@CGNonofr awesome. I will look at all the changes later today.

do you have an idea about the build error?

I am locally on Ubunut 24.04.1 and it fails at the same point with a slightly different reason. I will come back to you...

@kaisalmen
Copy link
Collaborator

kaisalmen commented Jan 21, 2025

@CGNonofr I upgraded volta npm to 22.13.0 (current LTS) and it works locally now so far. It went further but is not yet completed.

@kaisalmen
Copy link
Collaborator

kaisalmen commented Jan 21, 2025

@CGNonofr now it fails here:
image

I will install ajv as devDependency and see what happens.

Why is yarn still used?

@kaisalmen
Copy link
Collaborator

@CGNonofr if yarn is still needed you should also enforce the version with volta

@CGNonofr CGNonofr changed the title Reword build process and update to 1.96.3 Rewamp build process and update to 1.96.3 Jan 21, 2025
@CGNonofr
Copy link
Contributor Author

CGNonofr commented Jan 21, 2025

I am locally on Ubunut 24.04.1 and it fails at the same point with a slightly different reason. I will come back to you...

Interesting!

I upgraded volta npm to 22.13.0 (current LTS) and it works locally now so far. It went further but is not yet completed.

If you look at the commit history, you would notice that I've tried playing with the node/npm version already (:

if yarn is still needed you should also enforce the version with volta

I was thinking the whole repo has switched from yarn to npm, but maybe not everywhere? 🤔

I've failed to have any issue locally, and I've failed to have any other error then the npm warn cleanup Failed to remove some directories error, no mattern the npm and node version, or if the cache is cleaned..

@kaisalmen
Copy link
Collaborator

kaisalmen commented Jan 21, 2025

nice! there is another error about ajv though 🤔

That is the same I see locally. Can you patch the vscode package.json to contain those package (ajv and ajv-keywords)?

@kaisalmen
Copy link
Collaborator

@CGNonofr Getting there...
npm i ✅
npm run build ❌

@CGNonofr CGNonofr force-pushed the lmn/update-vscode-1.96 branch 4 times, most recently from 9290a3b to bef41fd Compare January 21, 2025 15:08
@CGNonofr
Copy link
Contributor Author

@kaisalmen this time I'm confident :)

@kaisalmen
Copy link
Collaborator

this time I'm confident :)

🤞

@CGNonofr
Copy link
Contributor Author

this time I'm confident :)

🤞

demo failed !!

@kaisalmen
Copy link
Collaborator

kaisalmen commented Jan 21, 2025

I'm aware it's a HUGE PR, sorry for that, I don't really expect you to fully understand what has been done about the build process as it would be really time consuming.

Yes, it is much, but I can already see that it is better structured.

I've decided that only external dependencies imported from actual ESM files are declared in the package dependencies. External dependencies only imported from d.ts files are put in the peerDependencies as optional (using peerDependenciesMeta). Any though about it?

This sounds logical. Is this also the reason why you use verbatimModuleSyntax now. I didn't know this existed. I assume this will make the dependency handling in projects using monaco-vscode-api more robust. 👍

You switched to used tsx because ts-node does no longer evolve, right. Is that similar tom vite-node?

Once the build is stable and you can produce a new release, I will test it with monaco-languageclient.

@CGNonofr
Copy link
Contributor Author

Is this also the reason why you use verbatimModuleSyntax now.

We don't really need it, it was part of an experiment that I don't use now. But It looks like a good practice so I've kept it.

You switched to used tsx because ts-node does no longer evolve, right.

I've discovered tsx on another project after fighting with ts-node for hours, and tsx was fully working on the first try! Also ts-node always generates verbose useless warning messages

@CGNonofr CGNonofr force-pushed the lmn/update-vscode-1.96 branch from 124ec32 to 9b4b750 Compare January 21, 2025 16:46
@CGNonofr
Copy link
Contributor Author

CGNonofr commented Jan 21, 2025

@kaisalmen All green 🎉 Thanks for your help!

@CGNonofr CGNonofr changed the title Rewamp build process and update to 1.96.3 Revamp build process and update to 1.96.3 Jan 21, 2025
@kaisalmen
Copy link
Collaborator

All green 🎉 Thanks for your help!

Wonderful. Did you already test the release process?

Copy link
Collaborator

@kaisalmen kaisalmen left a comment

Choose a reason for hiding this comment

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

Let's go. We have to test this in the field. 🎉

@CGNonofr
Copy link
Contributor Author

All green 🎉 Thanks for your help!

Wonderful. Did you already test the release process?

It's always hard to test that, but the only difference is the subpackage path so I hope nothing broke 🤞

@CGNonofr CGNonofr merged commit d2b1b4f into main Jan 21, 2025
2 checks passed
@CGNonofr CGNonofr deleted the lmn/update-vscode-1.96 branch January 21, 2025 22:18
@kaisalmen
Copy link
Collaborator

@CGNonofr I saw that the creation of releases failed. Can you ping me once they are available? Thank you.

Copy link

🎉 This PR is included in version 12.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@CGNonofr
Copy link
Contributor Author

@kaisalmen You were already pinged :)

@kaisalmen
Copy link
Collaborator

@CGNonofr dependency problems, see: #557

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

Successfully merging this pull request may close these issues.

2 participants