angular / angular-cli

CLI tool for Angular
https://cli.angular.io
MIT License
26.68k stars 11.98k forks source link

Providing a DI token in commonEngine.render() does not provide its value to application #26323

Open Ozzcer opened 8 months ago

Ozzcer commented 8 months ago

Which @angular/* package(s) are the source of the bug?

Don't known / other

Is this a regression?

Yes

Description

Prior to upgrading to v17 our application passed express' req/res as REQUEST/RESPONSE DI Tokens through the render function of the engine in server.ts. Since upgrading to v17 from v16 these DI Token values are always null.

This can be seen in the minimal reproduction where all three of APP_BASE_HREF / REQUEST / RESPONSE are null in the console output while running 'ng serve'.

If using @angular-devkit/build-angular:browser instead of @angular-devkit/build-angular:application this problem does not occur. I assume there is a different way to provide dependencies from server.ts now but I have not been able to find any updated documentation on it.

Please provide a link to a minimal reproduction of the bug

https://github.com/Ozzcer/angular-ssr-di-issue

Please provide the exception or error you saw

Console outputs null for all DI Tokens provided in server.ts

Please provide the environment you discovered this bug in (run ng version)

Angular CLI: 17.0.0
Node: 18.17.1
Package Manager: npm 9.6.7
OS: linux x64

Angular: 17.0.2
... animations, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, platform-server
... router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1700.0
@angular-devkit/build-angular   17.0.0
@angular-devkit/core            17.0.0
@angular-devkit/schematics      17.0.0
@angular/cli                    17.0.0
@angular/ssr                    17.0.0
@schematics/angular             17.0.0
rxjs                            7.8.1
typescript                      5.2.2
zone.js                         0.14.2

Anything else?

No response

JeanMeche commented 8 months ago

Hi, have a look at #26175 which updated the schematics to import create the tokens that aren't exported anymore. You will need to provide the tokens yourself.

Ozzcer commented 8 months ago

Hi @JeanMeche , tokens are defined under express.tokens.ts in minimal reproduction (https://github.com/Ozzcer/angular-ssr-di-issue/blob/c08049cc39e2993eff903c29902826a1aa1f7d10/src/express.tokens.ts), this file was created automatically by 'ng update' in our original project and I copied the file over to the minimal reproduction.

Also APP_BASE_HREF is coming through as null and I thought that didn't need to be manually defined.

alan-agius4 commented 8 months ago

This is expected as for ng serve the server.ts is not used.

Whilst we can provide an optional request token, this will be not be same interface of the Express tokens as the dev-server does not use express. Also the mentioned tokens will not available when using prerendering.

The recommended approach to treat the tokens set in server.ts as optional.

Ozzcer commented 8 months ago

Ah ok, got it working now, I have had to run build and run server.mjs independently. Is there a way to get the dev-ssr-server to work with the application output / is there an equivalent builder available for application output? Or do we still need to use dev-ssr-server with seperate server + browser builder instead of application? The current documentation is quite confusing as it appears like running the application builder through ng serve should allow local ssr rendering

alan-agius4 commented 8 months ago

ng serve does allow SSR, but it does not use the server.ts, Instead it has it's own middlewares and server instance. You should definitly update the docs to make this clear.

SaeedDev94 commented 8 months ago

ng serve does allow SSR, but it does not use the server.ts, Instead it has it's own middlewares and server instance. You should definitly update the docs to make this clear.

This is a serious issue IMO, we should be able to inject REQUEST and RESPONSE with ng serve
How we can debug our app then ?
By building and running server.mjs in dist folder ?!
So server.ts is used only for production ?? Or am I missing something ?? I will continue using old ssr-dev-server for now
(I remember that in v16 and below, server.ts was used for both development and production. Why shouldn't it be the same as before? Or if we can use server.ts for both envs + esbuild, how can we do that?)

Enngage commented 8 months ago

I've been confused by this as well. I was hoping there is an alternative way for ssr-dev-server in the application builder so that we can both develop the app, have it rebuild on the fly and just use easily.

Since the server.ts is not used for ng serve, it also means that none of the custom API routes that are defined are registered. What is the recommended approach here?

eneajaho commented 8 months ago

One other use-case where running server.ts in dev mode (ng serve) would be to be able to change providers.

Ex. In browser I want to use normal assets loading, while in SSR mode I want to be able to read from file-system and inline icons for better perf. Usecase: https://github.com/push-based/ngx-fast-svg#ssr-usage

I don't see any way to make that work with the current setup.

SaeedDev94 commented 8 months ago

Imagine we created a fresh new (SSR) project with latest version of @angular/cli and we want to make sure that angular ssr server returns 404 HTTP Status Code instead of 200 for unregistered routes or we want to redirect a path to another based on some conditions only, so we should set 301 HTTP Status Code and ... We definitely need server response object to set the right HTTP Status Code (and also request for other use cases)
My question is: How we can do it in development environment ?!

oriollpz commented 8 months ago

Any solution on that?

Ozzcer commented 8 months ago

Not a fix for the issue but if you want to use esbuild found there is a browser-esbuild builder you can drop into an existing angular.json build. If, like myself, you do most development without SSR and then just test in SSR this is an alright method of getting the esbuild benefits in development, but it does warn that its temporary so hopefully before its removed we will be able to use application builder.

mkurcius commented 8 months ago

I have ran into same issue (confusion). I generated fresh app using angular 17, with --ssr option. Everything looks nice, ng serve was working, until I have a need to create new endpoint in server.ts. To my surprise my endpoint was not registered, and I was getting ERROR RuntimeError: NG04002: Cannot match any routes. URL Segment: 'api/user' Initially I though I made a mistake and start trying to fix it, and after sometime of failure after failure, I start realizing maybe server.ts was not part of ng serve?

It was confusing and I waste time trying to fix it.

It was super unintuitive, I had fresh app, generated by CLI, I've run app using ng serve which used default (generated) configuration and to found out server entrypoint was not working 🀯

SaeedDev94 commented 8 months ago

One of the request use cases: (user authentication)
Getting the cookie from browser request and add it to api request for protected pages like user profile (with interceptor) + allowing cookie header (angular/angular#15730) Now without request object we can't do that in dev env (SSR)
And imagine if we already have a guard for the routes !! we can't enter the user profile ...

FranzStrudel commented 8 months ago

I wasted so much time because it was indeed rendered server side but yet the request provider was empty.

The changelog of the last version (adding node run dist/xxx) put me on the path to this issue.

So confusing behavior.

It has to be at least clearly documented on the getting started page...

zygarios commented 8 months ago

In reference to my closed but related thread, I believe that the lack of use of server.ts in development mode is a very large regression. There are often things that we need to change there and be able to debug based on the RESPONSE and REQUEST objects. The configuration of this and the work on SSR has unnecessarily become more complicated. I hope that this will be somehow sensibly resolved because in its current form it is impossible for me to work with it on a daily basis and I am forced to use the classic approach in the Angular 16 version.

oriollpz commented 8 months ago

How do they expect to work authentication with cookies without access to REQUEST RESPONSE from the server.ts?

osnoser1 commented 8 months ago

I agree, in development mode, we should be able to get the request object to do, for example, authentication through the cookie.

This is a breaking change between v16 and v17, and I don't know if it was documented.

hudzenko commented 8 months ago

Our team has to keep v16 because of this issue as well. Ignoring custom server.ts in dev mode is a huge downgrade for apps with complex SSR.

JeanMeche commented 8 months ago

@hudzenko You can still update to v17 and keep using the browser (webpack) or browser-eslint builder.

zygarios commented 8 months ago

I still don't understand how I can achieve responsiveness without using REQUEST and the User-Agent contained therein. I have an application that uses hydration and has different components for the mobile view and others for the desktop view. However, without information about the device type on the server, I am forced to always serve the default view (mobile), which results in a strange layout change in the browser. Is there any other way to solve this than by using REQUEST?

SalathielGenese commented 8 months ago

A week going nuts & wondering if I am a fraud. I can now rest in peace :sob: I have been a bit of a mess since I stumbled on this.

My use case: hide Firebase from my clients, so I can just switch without refactoring the frontend (Firebase does not support region/zone fallbacks, so good for demo only).

Additionally, I want automatic state transfer for my i18n keys through the HTTP client (not custom craft as is the case with Firebase and Sanity).

Either way, I need that express server during development.

Platonn commented 8 months ago

This issue is a bummer also for 3rd party Angular library authors that depend on REQUEST/RESPONSE tokens. Now the library users that create a fresh Angular 17+ app (with application builder used by default), running SSR dev-server, face unexpected errors, because the injection tokens REQUEST/RESPONSE being not provided - becasue server.ts is not included in the SSR dev server run.

Note: I'm aware that since ng17, REQUEST/RESPONSE injection tokens need to be provided by the application authors themselves in server.ts, it's acceptable. What is disturbing the Developer Experience, is that server.ts is not included at all when running the (Vite) SSR dev server, therefore REQUEST/RESPONSE tokens have no chance to work.

I respect and admire all the work you, the Angular core team, are doing to help improve DX around SSR and performance!! πŸ™ At the same time, I'm hoping we can avoid introducing an unintended regression to DX by excluding server.ts file from the SSR dev server run, if possible.

Again, thanks for keeping this framework great and developing it even better!

shady-ahmed-bdr commented 7 months ago

at first i thought that much better : fast build time frontend and backend(ssr) all with one command ng serve, now that I understand that vite makes its own server XD which makes me wonder what the point of SSR if I can't INTERACT WITH IT!!! (server.ts). of course with all respect <3 Angular

fluoxa commented 7 months ago

Yeah, devmode without using server.ts is also a nogo for our team.

We use the express framework for different things. For example to interact with cookies, redirects and integrate a SSO. I hope you can find a solution for that.

Thanks for your effords and the great framework.

nicolae536 commented 7 months ago

Unfortunately this is also the case for us πŸ€” Another things which I noticed πŸ€”

  1. For example when you build in prod mode you move assets into dist. But running ssr in dev mode does not move them. Example use case: Our app preloads all material icons in memory so that we can add the svg icons to the registry and when ssr goes we render them without reading from disk we have them cached in a hasmap and we can inline them directly in the html file.

  2. We ship our environment variables hardcoded inside the index.html file πŸ€” In order to do that get the build index where we have a special script tag and we inject the env vars there πŸ€” maybe we are not supposed to do this but this enables 1 build to be deployed in any environment without rebuilding the app again.

  3. We need access to index.html because we also replace the app bootstrapping scripts with a static script πŸ€” this helps us to have for example smarter cloudfront cache invalidation when we release new versions. To achieve this we remove the inlined scripts from index and inject 1 script which has a static name which bootstraps the app, with this small change we can invalidate only that script on cloudfront and not everything inside the distribution and the new app is basically served to everyone without problems and if somebody somehow accesses the old app the scripts are still there nothing will crash.

I can achieve 2 by altering the index when dev server starts πŸ€” but I think I cannot achieve 3 πŸ€” there what I do is when a request comes I run ssr I get the output -> remove the scripts from html and inject our bootstrap script which will call eval for every js in this index.html so I don't have a good idea if I can do this in the dev server now πŸ€” as long as it works in production I don't mind not doing this in the devserver but I need to know how I can support this use cases which we have.

Tom4U commented 7 months ago

I agree with all the others. Not using server.ts while running serve is pointless. Still I didn't want to do without Angular 17, as there are a couple of new features, that are worth it. So I was looking at the way, how to get around that issue.

A workaround for now, as a hint for others, would be running build command with development configuration and in parallel node dist/xxx/server.ts. Only pitfall is, you have to reload the browser yourselve, but at least it's close to serve.

Still hope, Angular team will have a look into this and I'm sure they find a solution.

JeanMeche commented 7 months ago

@Tom4U You can update to v17 and use either the browser or the browser-esbuild builders. Both work like before.

Burgov commented 7 months ago

@JeanMeche I hope this is somehow resolved before they remove those builders though

Tom4U commented 7 months ago

@Tom4U You can update to v17 and use either the browser or the browser-esbuild builders. Both work like before.

Yes, I know, thank you. But this would lead to use 5 instead of 2 builders again and though not investigated, probably would have to work with modules again. But I like to stay as close as possible to the new variant, as the other builders probably will fall out anyway in near future. And if setting up a new project, I don't like to back on an old horse. And the workaround is at least a good middle way, I guess.

pierrebouteloup commented 7 months ago

Ah ok, got it working now, I have had to run build and run server.mjs independently. [...]

Hi,

We are facing the same issue with our application. I tried to reproduce your solution by cloning your repo to learn why it's not working with our app, but we can't even make your code working 😭

git clone https://github.com/Ozzcer/angular-ssr-di-issue.git
cd ./angular-ssr-di-issue/
npm install
ng build
node ./dist/ssr-injection-issue/server/server.mjs
@Component({
  selector: 'app-root',
  standalone: true,
  imports: [CommonModule, RouterOutlet],
  templateUrl: './app.component.html',
  styleUrls: ['./app.component.css'],
})
export class AppComponent {
  title = 'ssr-injection-issue';

  constructor(
    @Optional() @Inject(APP_BASE_HREF) private appBaseHref: string,
    @Optional() @Inject(REQUEST) private request: string,
    @Optional() @Inject(RESPONSE) private response: string
  ) {
    console.log('basehref', this.appBaseHref); // null
    console.log('request', this.request); // null
    console.log('response', this.response); // null
  }
}

image

Don't understand how to simply provide values from server to our templates. Our usecase is to replace a dumb page made in ejs like :

  // legacy code we want to replace with our angular app
  app.get('our-route', (req, res) => {
    res.render('our-view', {email: req.user.email}); // how make something equivalent with Angular 17 and @angular/ssr πŸ™
  });

Thks to the Angular Team for all the work !

ste-sal commented 6 months ago

Any thing new on this issue? Upgraded to v17 this week and had to update my code to use standalone routing and bootstrapApplication etc so this is the only error left. Trying to use the browser builder doesn't seem to work for me either.

The recommended approach to treat the tokens set in server.ts as optional.

I can treat it as optional because it's not heavily used but then I won't be able to test requests when isPlatformServer is true. Is this the intent going forward as a byproduct of using vite or something that will be added in a later version?

hiepxanh commented 6 months ago

https://github.com/angular/angular-cli/pull/26667#pullrequestreview-1781435147 since this already merge, which allow index.html can manipulate, then we can add some thing like: server variable => index.html => angular read from this to client => then delete before we we send client if need. I think this is dirty hack for someone really need. I think it call "Embedding Variables in HTML"

seimic commented 6 months ago

Huh, same here :(

To get rid of some static environment profiles and to use configurations on server I tried following:

Simplified example:

Declare an injection token

export const FOO_TOKEN = new InjectionToken<string>('FOO');

than in server.ts

server.get('*', (req, res, next) => {
  ...
  commonEngine
    .render({
      ...
      providers: [
        { provide: FOO_TOKEN, useValue: 'whatever' } <-- Value provided
      ],
    })
    .then((html) => res.send(html))
    .catch((err) => next(err));
}

in a service or in app.component.ts

const fooKey = makeStateKey<string>("foo");

export class AppComponent implements OnInit {
  constructor(
    @Inject(PLATFORM_ID) private readonly platformId: Object,
    @Optional() @Inject(FOO_TOKEN) private readonly foo: string, // Never injected
    private readonly transferState: TransferState) {}

  ngOnInit(): void {
    if (isPlatformServer(this.platformId)) {
      this.transferState.set(fooKey, this.foo); // Undefined
    } else if (isPlatformBrowser(this.platformId)) {
      const foo = this.transferState.get(fooKey, 'cry now!');  // Undefined
      console.log('client: ' + foo);
    }
  }
}

Is this a bug or are the providers not intended to be used for such purposes? Do I need to register the tokens passed to common engine elsewhere too?

Kind regards, Michael

kewur commented 6 months ago

it's definitely a weird design choice where development and production code is forced to be different. are there any plans to change this behavior? debugging with npm run project/dist/server is hacky at best

BeCaDRI commented 5 months ago

So there are currently two issues discussed here?

  1. The general "no providers are available on client side"
  2. The DEV build does not use the server.ts?

right? Because a lot of the discussion is mixed up right now. I'm not able to provide any data to the app from server side, in DEV or PROD mode at all. Or are there any working example with Angular 17.1 and SSR? Example: https://github.com/angular/angular/issues/54170

JasonBStudee commented 5 months ago

So there are currently two issues discussed here?

  1. The general "no providers are available on client side"
  2. The DEV build does not use the server.ts?

right? Because a lot of the discussion is mixed up right now. I'm not able to provide any data to the app from server side, in DEV or PROD mode at all. Or are there any working example with Angular 17.1 and SSR? Example: angular/angular#54170

That looks correct to me - although any provider I try to create doesn't get it's data passed through when injected even in PROD mode (where server.ts is use) on the server side

salztorte commented 5 months ago

The problem is not if the PROD mode is enabled or not. It is just, that a ng serve don't use the server.ts.

juanmluces commented 5 months ago

Do we have any confirmation if the ng serve not using server.ts is a bug or a feature?

jokes aside

really, is it a known bug or its designed that way for a reason?

Ive had to stop developing in ssr yesterday because of this, there is no wey to get my Bearer Token form the cookies in the ssr first api call...

zygarios commented 5 months ago

A few months have passed and unfortunately, migrating to Angular version 17 is still not possible in our company, despite the fact that there is theoretically time for it. The version without SSR works great, but unfortunately we need SSR and rely on REQ and RES during development. It would be good if the creators at least gave a signal that they understand the problem and are looking for a solution or something. This lack of information is not optimistic.

I hope that you will quickly deal with this and allow us to use the amazing new features of Angular

Burgov commented 5 months ago

@zygarios as long as you don't migrate to the new builder, everything will work as it used to.

I'm just scared they're going to remove the old builder in a future version, without providing a proper solution to this problem

SaeedDev94 commented 5 months ago

@zygarios as long as you don't migrate to the new builder, everything will work as it used to.

I'm just scared they're going to remove the old builder in a future version, without providing a proper solution to this problem

What about new developers and new projects that they don't know there is a such workaround at all (old ssr engine) ?
+ The weak documentations about this topic?
+ 2 different envs for development and production?
+ The old engine is really slow compared to the new one

zygarios commented 5 months ago

@SaeedDev94 I agree, and what's more, it's not at all obvious. During the upgrade from version 16=>17, it is automatically migrated to devkit-angular:application, so despite the migration, you have to manually revert the changes. And using the old builder is problematic because nguniversal has stopped at version 16, which means we are forced to install it permanently with --force until this is fixed.

marosite commented 5 months ago

Is the issue scheduled? Can you tell us approximately when we can expect a fix? This is a very important matter for us. Thank you.

CarlosTorrecillas commented 5 months ago

We are also facing issues when trying to perform 301s that we used to prior the migration to Angular 17.1.3. In reality we are having two issues:

Our configuration for the server.ts is quite straightforward:

` // All regular routes use the Angular engine server.get('*', (req, res, next) => { const { protocol, originalUrl, baseUrl, headers } = req;

commonEngine
  .render({
    bootstrap,
    documentFilePath: indexHtml,
    url: `${protocol}://${headers.host}${originalUrl}`,
    publicPath: browserDistFolder,
    providers: [
      { provide: APP_BASE_HREF, useValue: baseUrl },
      { provide: RESPONSE, useValue: res },
      { provide: REQUEST, useValue: req }
    ],
  })
  .then((html) => res.send(html))
  .catch((err) => next(err));

});`

CarlosTorrecillas commented 5 months ago

@JeanMeche could you provide an example with the latest Angular 17 and using the browser setup? I am not able to get that working?

alan-agius4 commented 5 months ago

Thank you all for the additional details. Just to reiterate, this issue pertains specifically to ng serve.

We'll investigate this issue and ensure to keep you updated on any developments in this thread.

kamilpiusvs commented 5 months ago

It does not work in production mode for me neither.

alan-agius4 commented 5 months ago

@kamilpiusvs, in that case you are likely setting up DI incorrectly. Please open a new issue if the problem persists.

BeCaDRI commented 5 months ago

@alan-agius4 this

Thank you all for the additional details. Just to reiterate, this issue pertains specifically to ng serve.

We'll investigate this issue and ensure to keep you updated on any developments in this thread.

This is good to hear, but anyway all issues which were opened here on Github concerning the not working DI Token in production were closed with a reference to this thread. e.g.: https://github.com/angular/angular/issues/54170

So without the Cookie workaround from @gnibeda:

Has anyone a working example with Angular 17, the new builder and a custom token provided in SSR? Just a single one which can be shared?

alan-agius4 commented 5 months ago

I'm uncertain about @gnibeda's setup since there's no minimal reproduction of the issue. However, I would advise against implementing the changes suggested in https://github.com/angular/angular-cli/issues/26323#issuecomment-1936608857, as they rely on process.argv and filenames, which can be fragile. If you're encountering dependency injection issues outside of ng serve, please open a new issue and provide a minimal runnable reproduction. It's important to note that issues lacking a minimal runnable reproduction will be closed.