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

[Clover] jsdom: Mocks for element scrolling methods are missing #2108

Closed
akaufmann opened this issue May 14, 2021 · 7 comments · Fixed by #2117
Closed

[Clover] jsdom: Mocks for element scrolling methods are missing #2108

akaufmann opened this issue May 14, 2021 · 7 comments · Fixed by #2117

Comments

@akaufmann
Copy link

akaufmann commented May 14, 2021

I'm not sure if Clover is at a stage where you accept issues/PRs 😃 , so let me know if we should wait until you are over a certain point.

But one of the problems† I see is that some scrolling methods are not yet implemented in jsdom like scrollTo.
https://github.com/jsdom/jsdom/blob/master/lib/jsdom/browser/Window.js#L900

Which causes the server build to fail if a lib uses such a method.

Error: Not implemented: window.scrollTo
    at module.exports (<path/to/project>/node_modules/jsdom/lib/jsdom/browser/not-implemented.js:9:17)
    at <path/to/project>/node_modules/jsdom/lib/jsdom/browser/Window.js:866:7
    at rt.scrollToPosition (http://http/main.b51ee99a0a5badf2c7a5.js:1:111539)
    at c._next (http://http/main.b51ee99a0a5badf2c7a5.js:1:513321)
    at c.__tryOrUnsub (http://http/main.b51ee99a0a5badf2c7a5.js:1:1113718)
    at c.next (http://http/main.b51ee99a0a5badf2c7a5.js:1:1112947)
    at u._next (http://http/main.b51ee99a0a5badf2c7a5.js:1:1112126)
    at u.next (http://http/main.b51ee99a0a5badf2c7a5.js:1:1111900)
    at t.next (http://http/main.b51ee99a0a5badf2c7a5.js:1:1109640)
    at triggerEvent (http://http/main.b51ee99a0a5badf2c7a5.js:1:498683) undefined

There is already an issue filled...
jsdom/jsdom#1422

...with an open PR:
jsdom/jsdom#2626

The question is: Should we mock it for Clover for now or wait and see?
For example:

// server-engine.ts
 dom = new JSDOM(htmlContent, {
        runScripts: 'dangerously',
        resources: customResourceLoader,
        url: options.url,
        referrer: options.headers?.referrer as string | undefined,
        userAgent: options.headers?.['user-agent'] as string | undefined,
        beforeParse: (window) => {
          window.ngRenderMode = true;
          
          // TODO: remove it after https://github.com/jsdom/jsdom/pull/2626 is merged
          const noop = () => {};
          Object.defineProperty(window, 'scrollTo', { value: noop, writable: true });
          ...
        },
      });

† Another problem is routing.

🌍 Your Environment

Angular CLI: 12.0.0
Node: 12.18.0
Package Manager: yarn 1.22.10
OS: darwin x64

Angular: 12.0.0
... animations, cdk, cli, common, compiler, compiler-cli, core
... forms, language-service, material, platform-browser
... platform-browser-dynamic, platform-server, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1200.0
@angular-devkit/build-angular   12.0.0
@angular-devkit/core            12.0.0
@angular-devkit/schematics      12.0.0
@angular/fire                   6.1.4
@nguniversal/builders           12.0.0
@nguniversal/common             12.0.0
@schematics/angular             12.0.0
rxjs                            6.6.6
typescript                      4.2.4
@alan-agius4
Copy link
Collaborator

Thanks for the feedback, I think in this case it makes senses to noop these.

@alan-agius4
Copy link
Collaborator

I see you mean you had a problem with routing, is that so? And can you elaborate on this. Thanks

@akaufmann
Copy link
Author

When I build a (simple) app, the wildcard route is always rendered on the server.

const routes: Routes = [
  { path: '', loadChildren: () => import('./home/home.module').then(m => m.HomeModule) }, 
  { path: '**', loadChildren: () => import('./not-found/not-found.module').then(m => m.NotFoundModule) }
];

And on the client, Angular then replaces the <not-found>-component with the correct route content e.g. <home>.

@alan-agius4
Copy link
Collaborator

By rendered on the server do you mean when using server side rendering or prerendering?

@akaufmann
Copy link
Author

Yeah, sorry Alan, I mean it happens with SSR (haven't tested prerendering).

ng build cloverApp && ng run cloverApp:server && node dist/cloverApp/server/main.js

@alan-agius4
Copy link
Collaborator

I think I might know what’s happening. I’ll take a look at this earlier next week.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants