fullcalendar / fullcalendar-angular

The official Angular component for FullCalendar
https://fullcalendar.io/docs/angular
MIT License
1.08k stars 177 forks source link

Upgrade to Ivy package format #386

Closed Timebutt closed 1 year ago

Timebutt commented 3 years ago

This PR upgrades this repository to Angular 13. It also migrates everything to eslint 8.1.0 as TSLint was dropped from Angular. I verified all commands listed in package.json still work, AFAIK it should be fully functional.

Main advantage of this upgrade is that packages can finally be published as Ivy packages, removing the need for consuming apps to run ngcc!

DOrlov77 commented 2 years ago

Hello, the Angular 13 is here... Any plans to apply this fix?

yutao815 commented 2 years ago

Waiting for update

Timebutt commented 2 years ago

Hey @arshaw, I don't mean to be pushy about this, just hoping this PR catches your eye ;) Let me know if there is anything I can do to help get this merged faster?

arshaw commented 2 years ago

@Timebutt I'm very sorry for the delay. I'll review/merge/release it in the next week or two.

Timebutt commented 2 years ago

Awesome, curious to hear what your thoughts are ;)

nunoarruda commented 2 years ago

When running ng serve I get this:

β ™ Generating browser application bundles (phase: setup)...Processing legacy "View Engine" libraries:
- @ionic-native/core [module/esm5] (https://github.com/ionic-team/ionic-native.git)
- @ionic-native/onesignal [module/esm5] (https://github.com/ionic-team/ionic-native.git)
- ngx-sortablejs [es2015/esm2015] (https://github.com/sortablejs/ngx-sortablejs)
- ngx-material-timepicker [es2015/esm2015] (https://github.com/Agranom/ngx-material-timepicker.git)
- @ionic-native/ble [module/esm5] (https://github.com/ionic-team/ionic-native.git)
- @fullcalendar/angular [es2015/esm2015] (https://github.com/fullcalendar/fullcalendar-angular.git)
Encourage the library authors to publish an Ivy distribution.

My general understanding is that "View Engine" is deprecated and will eventually be removed and "Ivy" is the new next-level Angular compiler/packager. So I want to encourage you folks to publish an Ivy distribution of @fullcalendar/angular πŸ™‚

More info: https://angular.io/guide/creating-libraries https://blog.angular.io/upcoming-improvements-to-angular-library-distribution-76c02f782aa4 https://dev.to/this-is-angular/the-angular-ivy-guide-for-library-authors-9md

Timebutt commented 2 years ago

Hey @nunoarruda, that's exactly what this PR is for ;)

The core change to make this happen is this (along with upgrading to Angular 13, that enables this new setting):

"angularCompilerOptions": {
    "compilationMode": "partial"
  }
nunoarruda commented 2 years ago

Hey @Timebutt yesterday I noticed in a project that Angular 13.1 + @fullcalendar/angular 5.10.1 breaks lazy-loaded modules if those lazy-loaded modules import FullCalendarModule.

Wondering if you have any idea if this PR will also fix that.

Timebutt commented 2 years ago

Hey @nunoarruda, I'm facing the exact same problem with the latest Angular version. I'm not sure what's causing it yet, but I know this PR doesn't address the problem yet as I tried to use the build artifacts from my PR in my app and it still had the same problem :/

This is what I'm seeing, I'm guessing it's the same for you?

Warning: chunk default-node_modules_fullcalendar_angular___ivy_ngcc___fesm2015_fullcalendar-angular_js-node_-e7c5ea [mini-css-extract-plugin]
Conflicting order. Following module has been added:
 * css ./node_modules/css-loader/dist/cjs.js??ruleSet[1].rules[5].rules[0].oneOf[1].use[1]!./node_modules/@angular-devkit/build-angular/node_modules/postcss-loader/dist/cjs.js??ruleSet[1].rules[5].rules[0].oneOf[1].use[2]!./node_modules/@fullcalendar/daygrid/main.css
despite it was not able to fulfill desired ordering with these modules:
 * css ./node_modules/css-loader/dist/cjs.js??ruleSet[1].rules[5].rules[0].oneOf[1].use[1]!./node_modules/@angular-devkit/build-angular/node_modules/postcss-loader/dist/cjs.js??ruleSet[1].rules[5].rules[0].oneOf[1].use[2]!./node_modules/@fullcalendar/common/main.css
   - couldn't fulfill desired order of chunk group(s)

It also doesn't look like it's caused by @fullcalendar/angular, rather the daygrid and common in my case.

nunoarruda commented 2 years ago

On my side, after trying to navigate to the lazy-loaded route, I get this error:

GET http://localhost:4200/src_app_lazy-route_lazy-route_module_ts.js net::ERR_ABORTED 404 (Not Found)
ERROR Error: Uncaught (in promise): ChunkLoadError: Loading chunk src_app_lazy-route_lazy-route_module_ts failed.
(error: http://localhost:4200/src_app_lazy-route_lazy-route_module_ts.js)
ChunkLoadError: Loading chunk src_app_lazy-route_lazy-route_module_ts failed.
(error: http://localhost:4200/src_app_lazy-route_lazy-route_module_ts.js)
    at Object.__webpack_require__.f.j (jsonp chunk loading:27)
    at ensure chunk:6
    at Array.reduce (<anonymous>)
    at Function.__webpack_require__.e (ensure chunk:5)
    at loadChildren (app-routing.module.ts:4)
    at RouterConfigLoader.loadModuleFactory (router.mjs:3977)
    at RouterConfigLoader.load (router.mjs:3955)
    at router.mjs:3087
    at doInnerSub (mergeInternals.js:19)
    at outerNext (mergeInternals.js:14)
    at resolvePromise (zone.js:1213)
    at resolvePromise (zone.js:1167)
    at zone.js:1279
    at ZoneDelegate.invokeTask (zone.js:406)
    at Object.onInvokeTask (core.mjs:25437)
    at ZoneDelegate.invokeTask (zone.js:405)
    at Zone.runTask (zone.js:178)
    at drainMicroTaskQueue (zone.js:582)
    at ZoneTask.invokeTask [as invoke] (zone.js:491)
    at invokeTask (zone.js:1600)

I also noticed on the ng serve log that, before importing FullCalendarModule, the "Lazy Chunk Files" looks like this:

Lazy Chunk Files                           | Names                        |  Raw Size
src_app_lazy-route_lazy-route_module_ts.js | lazy-route-lazy-route-module |   5.46 kB |

After importing FullCalendarModule, the "Lazy Chunk Files" looks like this:

Lazy Chunk Files                                                             | Names                        |  Raw Size
src_app_lazy-route_lazy-route_module_ts.js, lazy-route-lazy-route-module.css | lazy-route-lazy-route-module | 595.32 kB | 

I believe that new/extra css file is the problem, this didn't happen with Angular 13.0. I will try to create a StackBlitz demo with this today and report to the Angular team. Smells like some issue on the Angular 13.1 side-of-things since fullcalendar was working fine with Angular 13.0.

Timebutt commented 2 years ago

@nunoarruda there seems to be a fix already here: https://github.com/angular/angular-cli/pull/22363. Not merged yet, but that seems to address the problem so you could use patch-package to fix your problem locally for now, or downgrade until they release this fix (original issue: https://github.com/angular/angular-cli/issues/22358).

blueiceprj commented 2 years ago

Ok. Thanks for your reply.

nunoarruda commented 2 years ago

FYI the lazy loading issue is fixed on Angular CLI v13.1.2

rahulraibit commented 2 years ago

There is also an issue with @fullcalendar/rrule plugin, its not working with angular v13.1.2. Is there a way to resolve it ?

image

Screenshot 2021-12-18 at 1 08 22 PM
Timebutt commented 2 years ago

Hey @arshaw, not sure if you have some time available in the near future to review this? I feel it's a pretty straightforward PR that brings a nice improvement for anybody already on Angular 13 🀞

NicolasKritter commented 2 years ago

On my side, after trying to navigate to the lazy-loaded route, I get this error:

GET http://localhost:4200/src_app_lazy-route_lazy-route_module_ts.js net::ERR_ABORTED 404 (Not Found)
ERROR Error: Uncaught (in promise): ChunkLoadError: Loading chunk src_app_lazy-route_lazy-route_module_ts failed.
(error: http://localhost:4200/src_app_lazy-route_lazy-route_module_ts.js)
ChunkLoadError: Loading chunk src_app_lazy-route_lazy-route_module_ts failed.
(error: http://localhost:4200/src_app_lazy-route_lazy-route_module_ts.js)
    at Object.__webpack_require__.f.j (jsonp chunk loading:27)
    at ensure chunk:6
    at Array.reduce (<anonymous>)
    at Function.__webpack_require__.e (ensure chunk:5)
    at loadChildren (app-routing.module.ts:4)
    at RouterConfigLoader.loadModuleFactory (router.mjs:3977)
    at RouterConfigLoader.load (router.mjs:3955)
    at router.mjs:3087
    at doInnerSub (mergeInternals.js:19)
    at outerNext (mergeInternals.js:14)
    at resolvePromise (zone.js:1213)
    at resolvePromise (zone.js:1167)
    at zone.js:1279
    at ZoneDelegate.invokeTask (zone.js:406)
    at Object.onInvokeTask (core.mjs:25437)
    at ZoneDelegate.invokeTask (zone.js:405)
    at Zone.runTask (zone.js:178)
    at drainMicroTaskQueue (zone.js:582)
    at ZoneTask.invokeTask [as invoke] (zone.js:491)
    at invokeTask (zone.js:1600)

I also noticed on the ng serve log that, before importing FullCalendarModule, the "Lazy Chunk Files" looks like this:

Lazy Chunk Files                           | Names                        |  Raw Size
src_app_lazy-route_lazy-route_module_ts.js | lazy-route-lazy-route-module |   5.46 kB |

After importing FullCalendarModule, the "Lazy Chunk Files" looks like this:

Lazy Chunk Files                                                             | Names                        |  Raw Size
src_app_lazy-route_lazy-route_module_ts.js, lazy-route-lazy-route-module.css | lazy-route-lazy-route-module | 595.32 kB | 

I believe that new/extra css file is the problem, this didn't happen with Angular 13.0. I will try to create a StackBlitz demo with this today and report to the Angular team. Smells like some issue on the Angular 13.1 side-of-things since fullcalendar was working fine with Angular 13.0.

I had the same problem, so I commented out all the import 'main.css" in the main.js file of each fullcalendar's component (daygrid, timeline...)

i added a script that does in postinstall and i can safely update to angular 13.1.2

To try if it works, remove the .angular folder before running ng serve.

No css problem so far after doing this

Timebutt commented 2 years ago

@NicolasKritter are these issues you are seeing in this branch of @fullcalendar/angular?

NicolasKritter commented 2 years ago

@NicolasKritter are these issues you are seeing in this branch of @fullcalendar/angular?

Sorry nope it is an issue with the current version, I just jumped into this pull request from an issue that reference this one, but as long as the import css is not fixed, the problem would be still there

In fact it is more related to your finding

It also doesn't look like it's caused by @fullcalendar/angular, rather the daygrid and common in my case.

As it is not only in daygrid but all comp

If you want I can move my comment proposing a dirty quick fix in another issue for the current version

arshaw commented 2 years ago

Hi all, I'm trying to merge this for the next release but I am encountering a problem. I can compile the @fullcalendar/angular project just fine. Here's the output of npm run build for me:

Building Angular Package

------------------------------------------------------------------------------
Building entry point '@fullcalendar/angular'
------------------------------------------------------------------------------
βœ” Compiling with Angular sources in Ivy partial compilation mode.
βœ” Writing FESM bundles
βœ” Copying assets
βœ” Writing package manifest
βœ” Built @fullcalendar/angular

------------------------------------------------------------------------------
Built Angular Package
 - from: /Users/adam/Code/fullcalendar/packages-contrib/angular/projects/fullcalendar
 - to:   /Users/adam/Code/fullcalendar/packages-contrib/angular/dist/fullcalendar
------------------------------------------------------------------------------

Build at: 2022-01-18T19:28:06.204Z - Time: 1500ms

However, when I try to use it within the Angular 12 example project (one major version down from 13) via npm-link, I get this error:

ERROR in The NgModule 'FullCalendarModule' in '/Users/adam/Code/fullcalendar/example-projects/angular/node_modules/@fullcalendar/angular/lib/fullcalendar.module.d.ts' is imported by this compilation, but appears to be part of a library compiled for Angular Ivy. This may occur because:

  1) the library was processed with 'ngcc'. Removing and reinstalling node_modules may fix this problem.

  2) the library was published for Angular Ivy and v12+ applications only. Check its peer dependencies carefully and ensure that you're using a compatible version of Angular.

See https://angular.io/errors/NG6999 for more information.

Is the @fullcalendar/angular package no longer compatible with Angular 12 projects after introducing this PR?

Timebutt commented 2 years ago

Hmm, it should be given we are using the partial compilation mode. I'll pull the code, set up an Angular 12 project and let you know what my experience is. Thanks for checking this!

arshaw commented 2 years ago

Another hurdle before releasing, I'd like an angular 13 example project. Can someone create one in an angular-13 subdirectory in this repo and submit a PR? I'd really appreciate it.

DavidCooper commented 2 years ago

Is there anything (perhaps small : ) that I could help move this forward @arshaw? I have a customer/project that really needs to move forward and the calendar is the lynch-pin.

NicolasKritter commented 2 years ago

@DavidCooper

Is there anything (perhaps small : ) that I could help move this forward @arshaw? I have a customer/project that really needs to move forward and the calendar is the lynch-pin.

I made a script on monkey-patch the problem (run after npm install : basically i just remove the import in all 'node_modules/@fullcalendar/{component used}/main.js

/* eslint-disable @typescript-eslint/no-var-requires */

const {
  readFileSync,
  writeFileSync,
  existsSync
} = require('fs');

const fix = '// ';

const patchTag = '//-patched';

const fileToPatch4 ='node_modules/@fullcalendar/timeline/main.js';
const fileToPatch5 ='node_modules/@fullcalendar/timegrid/main.js';
const fileToPatch2 ='node_modules/@fullcalendar/daygrid/main.js';

const fileToPatch ='node_modules/@fullcalendar/common/main.js';
const codeToPatch ='import \'./main.css\'';

const patch =fix+codeToPatch +patchTag;
function getAllIndexes(arr, val) {
  const indexes = [];
  let  i;
  for (i = 0; i < arr.length; i++)
  {if (arr[i].indexOf(val) !== -1)
  {indexes.push(i);}}
  return indexes;
}
// contents.findIndex(line => line.indexOf(sourceCode) !== -1)
function doPatch(fileName, sourceCode, patchCode, patchIdentifier) {
  if (!existsSync(fileName)) {
    console.log('file not found ' + fileName);
    return;
  }
  const contents = readFileSync(fileName).toString().split('\n');
  // Check if code has been patched already
  const hasBeenPatched = contents.find(line => line.indexOf(patchIdentifier) !== -1);

  if (!hasBeenPatched) {
    const lineNumbers = getAllIndexes(contents, sourceCode);
    if (lineNumbers.length < 1) {
      console.error('Could not find source code. Please check ' + fileName + ' and update the patch accordingly');
      return;
    }
    // replace the line
    lineNumbers.forEach((lineNumber) => {
      contents.splice(lineNumber, 1, patchCode);
    });
    const updatedContents = contents.join('\n');
    writeFileSync(fileName, updatedContents);

    console.log('Monkey patched');
  } else {
    console.log('already been patched');
  }
}
doPatch(fileToPatch, codeToPatch, patch, patchTag);
doPatch(fileToPatch2, codeToPatch, patch, patchTag);
doPatch(fileToPatch4, codeToPatch, patch, patchTag);
doPatch(fileToPatch5, codeToPatch, patch, patchTag);

hope it can help you move forward waiting for a solution (note that I use this patch on the released version of fullcalendar, not this branch and with angular 13.1.1

DavidCooper commented 2 years ago

Thanks @NicolasKritter, I will see if I can give it a try but ultimately I need to have an updated build from npm.

PooriaShariatzadeh commented 2 years ago

can I ask when we can have this pr on the main branch?

arshaw commented 2 years ago

@NicolasKritter, the angular maintainers restored the ability to do .css imports. If you are getting that error you might be using an old version.

The main hurdle to merging this PR still stands: the package that it produces is not backward compatible with Angular 12. I am still getting this error when trying to compile from an Angular 12 project.

I took the lazy approach and used the existing codebase, and merely bumped the dependencies:

  "peerDependencies": {
    "@angular/common": "9 - 13",
    "@angular/core": "9 - 13"
  },

Things work in both angular 12 and 13.

I've released this as @fullcalendar/angular version 5.10.2. Please install it and give it a try.

Some more work needs to be done on this PR to make it backward compatible with angular 12. I'm leaving it open and hope we can get it merged at some point. Seems like it provides good benefits, like being consumed via Ivy.

Timebutt commented 2 years ago

Hey @arshaw, sorry for not getting back earlier but I was hoping to get back to you with some news on backwards compatibility but from my investigation I just don't think it's possible. Any other Angular library I've seen that currently publishes an Ivy package is incompatible with any Angular version prior to 13. What you'll see, is they release a new major version of the package, clearly listing what Angular version is supported by what version of the package (example: ngx-quill). As such, I think releasing it as 5.10.2 is probably confusing, given that this is a breaking change. Maybe we should release this as 6.0.0 and update the docs to clearly mention the version support?

pclancysc commented 2 years ago

or v13 to keep in sync with angular version?

nunoarruda commented 2 years ago

Hey @arshaw, sorry for not getting back earlier but I was hoping to get back to you with some news on backwards compatibility but from my investigation I just don't think it's possible. Any other Angular library I've seen that currently publishes an Ivy package is incompatible with any Angular version prior to 13. What you'll see, is they release a new major version of the package, clearly listing what Angular version is supported by what version of the package (example: ngx-quill). As such, I think releasing it as 5.10.2 is probably confusing, given that this is a breaking change. Maybe we should release this as 6.0.0 and update the docs to clearly mention the version support?

@Timebutt in theory it can be compatible with Angular 10+ but depends on Angular version, application engine, and library engine (see https://gist.github.com/LayZeeDK/61caba93df1ec1a0788c94a973c8dfac).

Still, I believe you're mostly correct or on the right track. Like Minko said in a blog post:

Developers can use all these libraries in Ivy applications and libraries thanks to the compatibility compiler, but not vice versa β€” View Engine libraries can’t depend on Ivy.

So Ivy can be retro compatible with View Engine libraries but can't produce View Engine libraries and library authors might have to think like...

Is my library compiled with View Engine? If yes, then it can work on View Engine based or Ivy based apps but depends on Angular version and application engine.

Is my library compiled with Ivy? If yes, then it will only work on Ivy based apps.

So I agree with @Timebutt, release a new major version (6.0.0) with Ivy only support because it is a breaking change, and update the documentation to say that fullcalendar-angular is now a Ivy library compatible only with Angular 10+.

Note: Ivy is available since Angular 8. On Angular <=8 the default engine is View Engine, on Angular 9+ the default engine is Ivy. View Engine was deprecated on Angular 12 and removed on Angular 13.

arshaw commented 2 years ago

Just to be clear, the 5.10.2 version of @fullcalendar/angular I just released does not introduce any breaking changes. It is based on the current codebase (not this PR) and outputs a "View Engine" library, which is compatible with both "View Engine" and "Ivy" applications.

@nunoarruda, thanks for all the info. It's very helpful.

I'm realizing this PR outputs an Ivy library (because it removes this line and because enableIvy:true is the default). It seems like compilationMode:'partial' does NOT produce a library that is both Ivy and View Engine compatible, which is what I originally thought.

We need to continue to output a View Engine library if we want our library to be compatible with View Engine Angular projects. We could hypothetically still merge this PR, and reintroduce enableIvy:false to achieve this.

For the next major release of @fullcalendar/angular, version 6.0.0, I am comfortable outputting an Ivy library, which would require all consuming projects to be Ivy as well (the default). If anyone objects, please let me know.

Unfortunately, I decided to have FullCalendar's packages share the same major version number, to clearly indicate compatibility between FullCalendar plugins. Having an impromptu v6 for @fullcalendar/angular would defy that. Let me think about this more.

In the meantime, 5.10.2 version of @fullcalendar/angular is released and compatible with angular 12 AND 13.

nunoarruda commented 2 years ago

@arshaw that sounds reasonable.

@Timebutt maybe this PR could be renamed to something more explicit like "Publish an Ivy distribution" to avoid confusion with the notion of upgrading (peer) dependencies to Angular 13.

arshaw commented 2 years ago

@Timebutt, I'm trying to get this into the world. In reference to my previous comment, I've decided that, to avoid creating a breaking change in v5, and to avoid creating a new major version that would be out-of-sync with other plugins, we should create a new package called @fullcalendar/angular-ivy (similar to the @fullcalendar/vue3 package).

When FullCalendar v6 is released, it will become the standard @fullcalendar/angular.

I've attempted to do this in the ivy branch in this repo. It is building correctly. However, when I try to build this new example project against it, it gives errors like:

ERROR in ./node_modules/@fullcalendar/angular-ivy/fesm2015/fullcalendar-angular-ivy.mjs 79:26-34
Can't import the named export 'Calendar' from non EcmaScript module (only default export is available)

For some reason it does not see many of the imports as ESM. Do you happen to know why?

Timebutt commented 2 years ago

Hey @arshaw, sorry I didn't mean to come across as impatient when referencing this PR in my comment yesterday ;) I know you are waiting for the next major release to ship the Ivy package format.

I have updated my PR yesterday by the way, as there were conflicts and bumped some more dependencies while at it. As far as I know, it should be fully functional but I have not yet tested the changes I did yesterday. I'll get to it later today and report back, or fix the problem should I find one!

Timebutt commented 2 years ago

Hey @arshaw, I took a look at the example project you mentioned and figured out the problem.

The issue is that you are using version ~0.1000.6 of @angular-devkit/build-angular which is not compatible with the other dependencies in the project, or the Ivy compatible version of @fullcalendar/angular. I changed this to ~12.0.3 (matching your @angular/core and other related dependencies) and the build passed just fine!

benpsnyder commented 2 years ago

@Timebutt I made a PR to your PR :P .... my team and I are just trying this out. It might take some more work but we need this for our Angular 14 project. https://github.com/Timebutt/fullcalendar-angular/pull/1

See https://www.npmjs.com/package/@benpsnyder/fullcalendar-angular-ivy

Timebutt commented 2 years ago

@benpsnyder what do you mean you need this for your Angular 14 project? I'm currently running @fullcalendar/angular 5.11.2 in my Angular 14 app, albeit with a patch-package to prevent the build from failing on the import './main.css'; lines all @fullcalendar packages :/