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

Resources dependencies fixes and Vue 3 #17528

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Conversation

Skrypt
Copy link
Contributor

@Skrypt Skrypt commented Feb 25, 2025

  • Move old dependencies to Vendor folder and make sure that they get copied to wwwroot for reproducible builds.
  • Add Vue 3 and make it current dependency.
  • Removing Bootstrap 3 dependency as it was not even copied to the wwwroot folder anymore.
  • Rename vuedraggable to vue-draggable resource.
  • Migrate OrchardCore.Liquid GulpAssets.json to Assets.json using a run command.
  • Add Vue 3 vue-draggable and vue-multiselect new versions.
  • Fix yarn clean by always using Assets.json to move files to wwwroot folders.
    • Change also some "copy" actions to "min" in OrchardCore.Resource

This is the core changes required to start migrating to Vue 3 our components or to simply allow people to start creating new Vue 3 apps/components.


Also @sebastienros

We can run tsc directly with the new Asset Manager tool with the "run" action. See example used in this PR:

    {
        "action": "run",
        "name": "liquid-intellisense",
        "source": "Assets/monaco/",
        "order": 1,
        "scripts": {
            "build": "tsc liquid-intellisense.ts --outDir ../../wwwroot/monaco/ --target ES5 --removeComments true --lib dom,ES2015,es2015.collection,es2015.iterable,es2015.promise",
            "watch": "tsc liquid-intellisense.ts --outDir ../../wwwroot/monaco/ --target ES5 --removeComments true --lib dom,ES2015,es2015.collection,es2015.iterable,es2015.promise --watch"
        },
        "tags": ["admin", "js"]
    },

This is why there was no need to recreate a "typescript" only action. You can run the command shell line right there with all required params.

Here is the GulpAssets.json equivalent config:

  {
    "tsCompilerOptions": {
      "removeComments":  true,
      "target": "ES5",
      "lib": [
        "dom",
        "ES2015",
        "es2015.collection",
        "es2015.iterable",
        "es2015.promise"
      ]
    },
    "inputs": [
      "Assets/monaco/liquid-intellisense.ts"
    ],
    "output": "wwwroot/monaco/liquid-intellisense.js"
  }

As we can see it was not that easy to understand either based on the fact that you still need to know which configurations tsc has. Also, everything needed to be passed as a JSON object which we have literally no documentation about.

Move old dependencies to Vendor folder and make sure that they get copied to wwwroot for reproducible builds.
Add Vue 3 and make it current dependency.
Removing Bootstrap 3 dependency as it was not even copied to the wwwroot folder anymore.
Rename vuedraggable to vue-draggable resource.
@Skrypt Skrypt changed the title Resources rework Resources dependencies fix and Vue 3 Feb 25, 2025
@Skrypt Skrypt changed the title Resources dependencies fix and Vue 3 Resources dependencies fixes for Vue 3 Feb 25, 2025
@Skrypt Skrypt marked this pull request as ready for review February 25, 2025 07:08
@Skrypt Skrypt changed the title Resources dependencies fixes for Vue 3 Resources dependencies fixes and Vue 3 Feb 25, 2025
@Skrypt Skrypt marked this pull request as draft February 25, 2025 16:41
@Skrypt Skrypt marked this pull request as ready for review February 25, 2025 19:03
Fixes issue with build and watch not updating files
@Piedone
Copy link
Member

Piedone commented Feb 25, 2025

I'm sure it's awesome but I don't think I have the review of another multi-hundred file resource management PR in me so soon :). What does it fix (as oppose to improve), specifically, i.e. what was broken? The Bootstrap 3 reference, anything else?

@Skrypt
Copy link
Contributor Author

Skrypt commented Feb 25, 2025

Adds Vue 3 to the Resources and makes it possible to start using it. Other fixes are maintenance.
For some reasons the LiquidIntellisense.ts file started giving me error when compiling so I moved it to a "run" action that does exactly what it was doing. I think the issue is with ES5 instead of ES6 with that one.

@Piedone
Copy link
Member

Piedone commented Feb 25, 2025

Isn't that part of #14256?

@Skrypt
Copy link
Contributor Author

Skrypt commented Feb 25, 2025

Yes, but without migrating anything. At least we can start creating Vue 3 components/apps. I'm splitting this other Vue 3 PR because of that. The Vue 3 PR should only contain migrated apps and tweaks if any.

And also splitting because too many files makes the PR review impossible.

Copy link
Contributor

This pull request has merge conflicts. Please resolve those before requesting a review.

# Conflicts:
#	src/OrchardCore.Themes/TheAdmin/Assets.json
#	src/OrchardCore.Themes/TheAdmin/ResourceManagementOptionsConfiguration.cs
@Skrypt
Copy link
Contributor Author

Skrypt commented Feb 27, 2025

I'm sure it's awesome but I don't think I have the review of another multi-hundred file resource management PR in me so soon :). What does it fix (as oppose to improve), specifically, i.e. what was broken? The Bootstrap 3 reference, anything else?

This is exactly what makes commit PR's painful as a main contributor. I'm stuck at waiting on 2 approvals for basic things that should have been merged a year ago. 🤣

@Skrypt
Copy link
Contributor Author

Skrypt commented Feb 27, 2025

The goal is to make Orchard Core ready for Vue 3 and for me to stop merging latest changes from the main branch on my Vue 3 PR branch. There is way too much changes to sync on that branch every day making this take longer.

@Skrypt
Copy link
Contributor Author

Skrypt commented Feb 28, 2025

Here, also, I'm fixing yarn clean story to make sure assets gets copied to wwwroot folders. There was a lot of assets that were copied directly to the wwwroot folder of modules without using Assets.json file to make reproducible builds.

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.

2 participants