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

Rethink our JS/CSS build story (Gulp) #15740

Open
Piedone opened this issue Apr 12, 2024 · 22 comments
Open

Rethink our JS/CSS build story (Gulp) #15740

Piedone opened this issue Apr 12, 2024 · 22 comments
Milestone

Comments

@Piedone
Copy link
Member

Piedone commented Apr 12, 2024

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

Currently, we build static assets (JS/SCSS files) locally, and the build artifacts, i.e. the files in the wwwroot folders, are pushed to the repo. While this makes it easier to run OC locally, since you don't have to install and configure Node (which can be a PITA), it also has the following drawbacks:

  • We have to remember to run Gulp locally and we have to remind contributors too (and if we don't, mismatches between source and artifact files will be committed, which happens). This is why this problem also exists: Frontend assets are outdated #15726.
  • The files can be built differently between environments, thus causing diffs without actual changes.
  • If two people change two source files, there will always be a merge conflict in the artifacts.
  • We're needlessly storing build artifacts in the repo.

Describe the solution you'd like

We at Lombiq worked years on a proper client-side build system that's as seamless as possible in a .NET solution, integrating into .NET builds. First we did it with Gulp, then without any intermediaries, publishing Lombiq Node.js Extensions. After all of this experience, including making sure that dozens of people with varying skills can work with it, I'd recommend...

Not doing it. At all. It's an endless time sink, and the Node setup is something that'll always trip certain people, it'll always have issues in the myriad of ways it can be done on different platforms with different starting points. And with all the various tools added on top of it our dependency graph will be huge and we'll rely on a lot of community-maintained NPM packages that constantly fell out of favor (and that we have to maintain, see #15694, even if everything else is ideal).

With modern JS and CSS you can write good code without any build engine, like they do it as Basecamp. We don't really need SCSS, and we especially don't need JS files to be copied from one folder to another (which's the bulk of what our asset pipeline currently does). Since HTTP/2, bundling is not really necessary either, and if you use response compression (what you should) then minification isn't either.

Now, I'm only talking about builds. I'm a strong supporter of linting, which requires Node.js tools, but that's a slightly different story (and something that we can do even fully in CI, without contributors having to install anything).

Describe alternatives you've considered

Using Lombiq Node.js Extensions. That's viable, and a better route than using Gulp, but I don't think we need it.

@Skrypt
Copy link
Contributor

Skrypt commented Apr 12, 2024

Totally agree with using a proper build asset tool that can do reproducible builds. Remove these built assets from the repository and leave it to be built by developpers with a proper readme file that explains details. I don't know what Lombiq Node.js extensions does exactly but we are using a concurrent command line runner (currently) https://github.com/open-cli-tools/concurrently that replaces entirely the gulp pipeline and that is not opiniated on the bundler you want to use. We currently use Parcel with it and it can also work with Vite.js which is a wrapper around Rollup.js. I did add recently a "min" and "scss" action that does basically what the gulp pipeline does. It's fully extensible as it basically just run the commands that you want based on an Assets.json file just like Orchard does.

Alternatively there is also npm-run-all that is fully integrated with npm but it is less extensible and versatible than concurrently.

@Piedone
Copy link
Member Author

Piedone commented Apr 12, 2024

How about going #nobuild? :)

@Skrypt
Copy link
Contributor

Skrypt commented Apr 12, 2024

I'm not sure what you are suggesting here. We need to build .js files if they are ES6 modules like the GraphiQL editor. I'm not sure it is possible to have absolutely no build at all.

@Piedone
Copy link
Member Author

Piedone commented Apr 12, 2024

ES6 modules are supported in all modern browsers for about 4 years, you don't need build for that. Also see this article.

@Skrypt
Copy link
Contributor

Skrypt commented Apr 12, 2024

Well, the idea of a bundler is about doing tree shaking on @imports to not have them duplicated everywhere in the app. If you don't need external assets then you can do without a bundler else you will surely need one.

But, I agree with you that we should start using type="module" and write these script as ES6 modules.

@Skrypt
Copy link
Contributor

Skrypt commented Apr 12, 2024

image

@Skrypt
Copy link
Contributor

Skrypt commented Apr 12, 2024

There is a lot of inline scripts that could be converted to small ES6 modules and we could get rid of the document.ready story and also JQuery at the same time. We basically copy these from the Assets folder to the wwwroot folder just because we want to have a minimized version of them right now. I'm not sure about SCSS and native browser support though.

@Skrypt
Copy link
Contributor

Skrypt commented Apr 12, 2024

Oh well it is possible but what is the perf though? https://github.com/sass/dart-sass/blob/main/README.md#dart-sass-in-the-browser

@Skrypt
Copy link
Contributor

Skrypt commented Apr 12, 2024

Also after reading one of your pages I remembered using importmap once but we did not push on it.

https://developer.mozilla.org/fr/docs/Web/HTML/Element/script/type/importmap

I'm not sure how we would implement this as part of OC. We would need to rewrite the ResourceManager to write an importmap instead of adding script at foot with a dependency tree for example. This would work probably better than what we currently have.

@Piedone
Copy link
Member Author

Piedone commented Apr 12, 2024

Yeah, we'd lose tree-shaking (I'm not sure we actually have that currently though). However, I'd guesstimate that due to how we bundle scripts that a user might need, but doesn't need 100% of the time, even without this, we should be at least in the same place (due to on-demand script inclusions and dynamic imports). We could look into this in more detail.

As for SCSS, I mean that we don't need it, we can just write vanilla CSS.

I don't know import maps. Sounds interesting, but I wouldn't want to enforce having to write all scripts as modules though. But having the option, and this integrated into the Resource Manager would be great.

@Skrypt
Copy link
Contributor

Skrypt commented Apr 12, 2024

You can't really ... Bootstrap 5.

Sometimes concept are more like an utopia than an actual achievable goal... I don't know how you will avoid compilation entirely.

@Piedone
Copy link
Member Author

Piedone commented Apr 12, 2024

What do you mean with Bootstrap 5, what can't we do? BS now has CSS variables, so configuring basic settings are possible with vanilla CSS too. Mixins wouldn't be available, but I don't know if we need them much.

@Skrypt
Copy link
Contributor

Skrypt commented Apr 12, 2024

The entire Media Gallery is a Bootstrap 5 import of styles. We would need to use the BS5 .css everywhere and override it with a second .css instead. This would maybe grow the size of our resulting css.

Yeah, we use mixins in a couple places.

@Skrypt
Copy link
Contributor

Skrypt commented Apr 12, 2024

Your idea is great though. But I think it should only be used as an hybrid approach to lower CI build times. You will still need to build at some point; you can't remove it entirely.

@Piedone
Copy link
Member Author

Piedone commented Apr 12, 2024

Ah I see. We should weigh the options.

Copy link
Contributor

We triaged this issue and set the milestone according to the priority we think is appropriate (see the docs on how we triage and prioritize issues).

This indicates when the core team may start working on it. However, if you'd like to contribute, we'd warmly welcome you to do that anytime. See our guide on contributions here.

@Piedone
Copy link
Member Author

Piedone commented Nov 12, 2024

@MikeAlhayek I collected the quality fix issues for 2.1. Please don't reassign them to 2.x where they'll be among the many other issues, but to 2.2.

@Piedone Piedone modified the milestones: 2.x, 2.2 Nov 12, 2024
Copy link
Contributor

We triaged this issue and set the milestone according to the priority we think is appropriate (see the docs on how we triage and prioritize issues).

This indicates when the core team may start working on it. However, if you'd like to contribute, we'd warmly welcome you to do that anytime. See our guide on contributions here.

@MikeAlhayek
Copy link
Member

When we tag something with 2.1, it means it must be completed before we can roll out version 2.1. Similarly, tagging something with 2.2 indicates that it needs to be completed before the 2.2 release.

However, to better manage tasks that are important but not necessarily blockers for a release, we could introduce a new milestone, 2.x-priority. This would allow us to flag tasks as high-priority, ensuring they are given attention but won't prevent a release from moving forward if necessary.

@Piedone
Copy link
Member Author

Piedone commented Nov 12, 2024

We can always reschedule issue, or we can utilize the P* labels (or rather, have a single, simple blocker label or something).

@sebastienros
Copy link
Member

I (also) prefer to set them to 2.x (anytime) and give them a high priority (another label)

@MikeAlhayek MikeAlhayek modified the milestones: 2.2, 3.0 Nov 21, 2024
Copy link
Contributor

We triaged this issue and set the milestone according to the priority we think is appropriate (see the docs on how we triage and prioritize issues).

This indicates when the core team may start working on it. However, if you'd like to contribute, we'd warmly welcome you to do that anytime. See our guide on contributions here.

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

No branches or pull requests

4 participants