build(docs-infra): fixed broken stackblitz example of i18n module #42001

iRealNirmal · 2021-05-08T08:55:37Z

This commit fixes the broken stackblitz example of animation.

Closes 41838.

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

angular.io/guide/i18n live example is broken

Issue Number: #41838

What is the new behavior?

angular.io/guide/i18n live example issue if fixed

Does this PR introduce a breaking change?

Yes
No

Other information

iRealNirmal · 2021-05-08T08:58:24Z

@petebacondarwin can you help in review this ? Also can you add air-preview label so it can be previewed as well.

mary-poppins · 2021-05-08T12:17:22Z

You can preview 6ce58f6 at https://pr42001-6ce58f6.ngbuilds.io/.
You can preview bd1b653 at https://pr42001-bd1b653.ngbuilds.io/.

petebacondarwin

I thought the fix was to add compiler-cli into the array here:

angular/aio/tools/stackblitz-builder/builder.js

Lines 49 to 51 in da6ed15

    
           // Add unit test packages from devDependencies for unit test examples 
        
           const devDependencies = packageJson.devDependencies; 
        
           ['jasmine-core', 'jasmine-marbles'].forEach(dep => exampleDependencies[dep] = devDependencies[dep]);

Did you try that?

iRealNirmal · 2021-05-08T12:51:08Z

I thought the fix was to add compiler-cli into the array here:

angular/aio/tools/stackblitz-builder/builder.js

Lines 49 to 51 in da6ed15

// Add unit test packages from devDependencies for unit test examples

const devDependencies = packageJson.devDependencies;

['jasmine-core', 'jasmine-marbles'].forEach(dep => exampleDependencies[dep] = devDependencies[dep]);

Did you try that?

No let me try and update but fyi this is common place so it will add for all files, are you fine with that ?

iRealNirmal · 2021-05-08T13:21:19Z

I thought the fix was to add compiler-cli into the array here:

angular/aio/tools/stackblitz-builder/builder.js

Lines 49 to 51 in da6ed15

// Add unit test packages from devDependencies for unit test examples

const devDependencies = packageJson.devDependencies;

['jasmine-core', 'jasmine-marbles'].forEach(dep => exampleDependencies[dep] = devDependencies[dep]);

Did you try that?

@petebacondarwin tried suggestion you provided but it's not working

petebacondarwin · 2021-05-08T15:15:31Z

The 422 error message helps to solve the mystery:

"MISSING_PEERS\",\"data\":{\"typescript\":{\"@angular/compiler-cli@11.2.1}

So we need both @angular/compiler-cli and typescript.

iRealNirmal · 2021-05-08T15:18:36Z

The 422 error message helps to solve the mystery:
"MISSING_PEERS\",\"data\":{\"typescript\":{\"@angular/compiler-cli@11.2.1}
So we need both @angular/compiler-cli and typescript.

Yes you want me to import both in builder.js and try, or keep current approach ?

petebacondarwin · 2021-05-08T15:20:12Z

Rather than include these dependencies in every StackBlitz, I propose that we add a new property to the stackblitz.json config file that allows us to specify devDependencies to include in the dependencies of the StackBlitz.

So for the i18n example we would have:

{
    "description": "i18n",
    "files":[
      "!**/*.d.ts",
      "!**/*.js",
      "!**/*.[0-9].*",
      "!doc-files/**/*",	
      "**/*.xlf"
    ],
    "file": "src/app/app.component.ts", 
    "tags": ["Angular", "i18n", "internationalization"],
    "devDependencies": ["@angular/compiler-cli", "typescript"]
}

and then for the testing example we would have:

{
  "description": "Heroes Test App",
  "files":[
    "src/index.html",
    "src/main.ts",
    "src/styles.css",
    "src/test.css",

    "e2e/src/**/*.ts",

    "src/app/**/*.css",
    "src/app/**/*.html",
    "src/app/**/*.ts",

    "!src/**/*.spec.ts"
  ],
  "tags": ["testing"],
  "devDependencies": ["jasmine-core", "jasmine-marbles"]
}

And then we can remove the hard coded items from builder.js.

mary-poppins · 2021-05-09T03:34:46Z

You can preview ebc1e1a at https://pr42001-ebc1e1a.ngbuilds.io/.

iRealNirmal · 2021-05-09T04:26:01Z

Rather than include these dependencies in every StackBlitz, I propose that we add a new property to the stackblitz.json config file that allows us to specify devDependencies to include in the dependencies of the StackBlitz.

So for the i18n example we would have:

{
    "description": "i18n",
    "files":[
      "!**/*.d.ts",
      "!**/*.js",
      "!**/*.[0-9].*",
      "!doc-files/**/*",	
      "**/*.xlf"
    ],
    "file": "src/app/app.component.ts", 
    "tags": ["Angular", "i18n", "internationalization"],
    "devDependencies": ["@angular/compiler-cli", "typescript"]
}

and then for the testing example we would have:

{
  "description": "Heroes Test App",
  "files":[
    "src/index.html",
    "src/main.ts",
    "src/styles.css",
    "src/test.css",

    "e2e/src/**/*.ts",

    "src/app/**/*.css",
    "src/app/**/*.html",
    "src/app/**/*.ts",

    "!src/**/*.spec.ts"
  ],
  "tags": ["testing"],
  "devDependencies": ["jasmine-core", "jasmine-marbles"]
}

And then we can remove the hard coded items from builder.js.

@petebacondarwin it was good suggestion, I have done changes you suggested. can you re-review and also help to rerun test_ivy_aot_win ?

petebacondarwin

Thanks for the update. I think it can be a lot simpler.

petebacondarwin · 2021-05-09T16:40:50Z

aio/tools/stackblitz-builder/builder.js

@@ -100,7 +121,7 @@ class StackblitzBuilder {
    try {
      const config = this._initConfigAndCollectFileNames(configFileName);
      const postData = this._createPostData(config, configFileName);
-      this._addDependencies(config, postData);
+      this._addDependencies(config, postData, configFileName);


No need to add a new parameter here. The config parameter already contains the devDependencies that you need.

petebacondarwin · 2021-05-09T16:42:53Z

aio/tools/stackblitz-builder/builder.js

+    const testStackblitzestPath = path.join(this.basePath, '/testing/stackblitz.json');
+    const stackblitzJsonDevDependencies = this._getDevDependencies(stackBlitzJson);
+    const stackblitzJsonTestDependencies = this._getDevDependencies(testStackblitzestPath);


I'm not sure why this code is here, and why it is hard-coded to the /testing/ path.

@petebacondarwin this seems required change because if I remove this it will not import
"jasmine-core", "jasmine-marbles" from test file.

If we don't want to add this line of code then other alternative would be to add
"jasmine-core", "jasmine-marbles" it each and every stackblitz.json

For now I have done changes as you suggested and you can see difference in live example between
https://pr42001-ebc1e1a.ngbuilds.io/guide/i18n
https://pr42001-335a4ca.ngbuilds.io/guide/i18n

aio/tools/stackblitz-builder/builder.js

mary-poppins · 2021-05-10T02:34:43Z

You can preview 335a4ca at https://pr42001-335a4ca.ngbuilds.io/.

This commit fixes the broken stackblitz example of i18n. Closes angular#41838.

mary-poppins · 2021-05-10T07:32:45Z

You can preview 7d7abc9 at https://pr42001-7d7abc9.ngbuilds.io/.

petebacondarwin

LGTM! Thanks @iRealNirmal

aio/content/examples/i18n/stackblitz.json

This commit fixes the broken stackblitz example of i18n. Closes #41838. PR Close #42001

angular-automatic-lock-bot · 2021-06-10T16:13:58Z

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.}

google-cla bot added the cla: yes label May 8, 2021

pullapprove bot requested a review from gkalpak May 8, 2021 08:55

iRealNirmal force-pushed the animation-stackblitz-dependency-issue branch from 2282516 to 6ce58f6 Compare May 8, 2021 08:57

iRealNirmal force-pushed the animation-stackblitz-dependency-issue branch from 6ce58f6 to bd1b653 Compare May 8, 2021 09:13

iRealNirmal changed the title ~~docs(animation): fixed animation broken stackblitz example~~ docs: fixed broken stackblitz example of animation module May 8, 2021

petebacondarwin added action: review The PR is still awaiting reviews from at least one requested reviewer aio: preview comp: docs target: patch This PR is targeted for the next patch release labels May 8, 2021

ngbot bot modified the milestone: Backlog May 8, 2021

petebacondarwin reviewed May 8, 2021

View reviewed changes

iRealNirmal force-pushed the animation-stackblitz-dependency-issue branch from bd1b653 to d0fcfd9 Compare May 9, 2021 03:18

pullapprove bot requested a review from IgorMinar May 9, 2021 03:18

iRealNirmal force-pushed the animation-stackblitz-dependency-issue branch 2 times, most recently from a8a3afe to ebc1e1a Compare May 9, 2021 03:22

petebacondarwin requested changes May 9, 2021

View reviewed changes

iRealNirmal force-pushed the animation-stackblitz-dependency-issue branch from ebc1e1a to 335a4ca Compare May 10, 2021 02:24

iRealNirmal force-pushed the animation-stackblitz-dependency-issue branch from 335a4ca to a9cc93e Compare May 10, 2021 07:20

iRealNirmal force-pushed the animation-stackblitz-dependency-issue branch from a9cc93e to b251ee8 Compare May 10, 2021 07:21

iRealNirmal changed the title ~~docs: fixed broken stackblitz example of animation module~~ build(docs-infra): fixed broken stackblitz example of i18n module May 10, 2021

build(docs-infra): fixed i18n broken stackblitz example

7d7abc9

This commit fixes the broken stackblitz example of i18n. Closes angular#41838.

iRealNirmal force-pushed the animation-stackblitz-dependency-issue branch from b251ee8 to 7d7abc9 Compare May 10, 2021 07:23

petebacondarwin removed the action: review The PR is still awaiting reviews from at least one requested reviewer label May 10, 2021

petebacondarwin approved these changes May 10, 2021

View reviewed changes

petebacondarwin requested review from AndrewKushnir and removed request for gkalpak May 10, 2021 07:52

petebacondarwin added the action: review The PR is still awaiting reviews from at least one requested reviewer label May 10, 2021

gkalpak reviewed May 10, 2021

View reviewed changes

aio/content/examples/i18n/stackblitz.json Show resolved Hide resolved

gkalpak mentioned this pull request May 10, 2021

build(docs-infra): add boilerplate to support i18n runtime example #41785

Closed

14 tasks

AndrewKushnir approved these changes May 10, 2021

View reviewed changes

petebacondarwin removed the request for review from IgorMinar May 10, 2021 15:50

petebacondarwin added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels May 10, 2021

alxhub closed this in e071e3b May 10, 2021

alxhub pushed a commit that referenced this pull request May 10, 2021

build(docs-infra): fixed i18n broken stackblitz example (#42001)

2d9a9aa

This commit fixes the broken stackblitz example of i18n. Closes #41838. PR Close #42001

alxhub pushed a commit that referenced this pull request May 10, 2021

build(docs-infra): fixed i18n broken stackblitz example (#42001)

150b2ec

This commit fixes the broken stackblitz example of i18n. Closes #41838. PR Close #42001

petebacondarwin mentioned this pull request May 16, 2021

stackblitz example of http are not working #42116

Closed

iRealNirmal mentioned this pull request May 17, 2021

build(docs-infra): fixed broken stackblitz examples #42122

Closed

14 tasks

angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build(docs-infra): fixed broken stackblitz example of i18n module #42001

build(docs-infra): fixed broken stackblitz example of i18n module #42001

iRealNirmal commented May 8, 2021

iRealNirmal commented May 8, 2021

mary-poppins commented May 8, 2021

petebacondarwin left a comment

iRealNirmal commented May 8, 2021

iRealNirmal commented May 8, 2021

petebacondarwin commented May 8, 2021

iRealNirmal commented May 8, 2021

petebacondarwin commented May 8, 2021

mary-poppins commented May 9, 2021

iRealNirmal commented May 9, 2021

petebacondarwin left a comment

petebacondarwin May 9, 2021

petebacondarwin May 9, 2021

iRealNirmal May 10, 2021

mary-poppins commented May 10, 2021

mary-poppins commented May 10, 2021

petebacondarwin left a comment

angular-automatic-lock-bot bot commented Jun 10, 2021

	// Add unit test packages from devDependencies for unit test examples
	const devDependencies = packageJson.devDependencies;
	['jasmine-core', 'jasmine-marbles'].forEach(dep => exampleDependencies[dep] = devDependencies[dep]);

build(docs-infra): fixed broken stackblitz example of i18n module #42001

build(docs-infra): fixed broken stackblitz example of i18n module #42001

Conversation

iRealNirmal commented May 8, 2021

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

iRealNirmal commented May 8, 2021

mary-poppins commented May 8, 2021

petebacondarwin left a comment

Choose a reason for hiding this comment

iRealNirmal commented May 8, 2021

iRealNirmal commented May 8, 2021

petebacondarwin commented May 8, 2021

iRealNirmal commented May 8, 2021

petebacondarwin commented May 8, 2021

mary-poppins commented May 9, 2021

iRealNirmal commented May 9, 2021

petebacondarwin left a comment

Choose a reason for hiding this comment

petebacondarwin May 9, 2021

Choose a reason for hiding this comment

petebacondarwin May 9, 2021

Choose a reason for hiding this comment

iRealNirmal May 10, 2021

Choose a reason for hiding this comment

mary-poppins commented May 10, 2021

mary-poppins commented May 10, 2021

petebacondarwin left a comment

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Jun 10, 2021