Skip to content
This repository was archived by the owner on Dec 18, 2024. It is now read-only.

fix(stack-blitz-tests): revert enabling Ivy as it broke harness examples #1009

Merged
merged 1 commit into from
Jul 29, 2021

Conversation

Splaktar
Copy link
Contributor

@Splaktar Splaktar commented Jun 7, 2021

  • remove tslint and codelyzer from examples
  • update jasmine dependencies
  • remove unused material-module.ts from test assets
  • add jasmine-core to package.json in stack-blitz-tests/
  • disable noImplicitAny in stack-blitz-tests' tsconfig.spec.json since
    jasmine-core/lib/jasmine-core/jasmine.js imported in test/jasemine-setup.ts
    is not compatible

Fixes #997

The stack-blitz-tests/ asset files are used for generating the StackBlitz examples for the test harnesses. In a past PR, we set them along with all of our StackBlitz assets to use Ivy. This worked fine for our component examples, but broke our Harness examples as mentioned in #997 (comment). This PR reverts that change by switching the Harness examples to use View Engine in StackBlitz. The component examples will continue to use Ivy in StackBlitz.

Additionally, this PR fixes some issues blocking the use of Ivy with the Harness examples (since jasmine-core/lib/jasmine-core/jasmine.js doesn't build in strict mode with noImplicitAny enabled). It also updates and removes some other dependencies.

@andrewseguin
Copy link
Collaborator

The material-module.ts is used by the stackblitz writer.

It's not clear why Ivy should be disabled - is there something that the framework needs to fix in order for us to turn it back on?

@Splaktar
Copy link
Contributor Author

Splaktar commented Jun 7, 2021

This workaround from Pete might work to keep Ivy enabled:
angular/angular#41999 (comment)

@Splaktar
Copy link
Contributor Author

Splaktar commented Jun 7, 2021

@andrewseguin not by assets/stack-blitz-tests, only by assets/stack-blitz. I.e. TEMPLATE_FILES uses it but TEST_TEMPLATE_FILES does not.

@Splaktar
Copy link
Contributor Author

Splaktar commented Jun 7, 2021

@andrewseguin also after fixing the noImplicitAny error, with Ivy enabled, you get this in StackBlitz

Error in src/app/input-harness-example.spec.ts (14:1)
Cannot find name 'describe'. Do you need to install type definitions for a test runner?
Try `npm i --save-dev @types/jest` or `npm i --save-dev @types/mocha`.

Disabling Ivy resolves this. It's unclear why there are references to Jest or Mocha when Jasmine is being used.

@gkalpak
Copy link
Member

gkalpak commented Jun 7, 2021

I don't know why the error happens, but the error message comes from TypeScript itself: https://github.com/microsoft/TypeScript/blob/9906092db25aae8bd6f6b04aef5e8906251fcf13/src/compiler/diagnosticMessages.json#L2489 🤷

@Splaktar
Copy link
Contributor Author

Splaktar commented Jun 7, 2021

This workaround from Pete might work to keep Ivy enabled:
angular/angular#41999 (comment)

@crisbeto unfortunately, that's not working for me in https://stackblitz.com/edit/jasmine-ivy-workaround?file=tsconfig.app.json.

@crisbeto
Copy link
Member

crisbeto commented Jun 7, 2021

Would everything else work if we put a @ts-ignore just on this one import?

@Splaktar
Copy link
Contributor Author

Splaktar commented Jun 7, 2021

@crisbeto that gets us past the noImplicityAny and on to the TypeScript error above about describe not being defined.

@Splaktar Splaktar requested review from wagnermaciel and removed request for andrewseguin June 10, 2021 16:47
- remove tslint and codelyzer from examples
- update jasmine dependencies
- remove unused material-module.ts from test assets
- add jasmine-core to package.json in stack-blitz-tests/
- disable `noImplicitAny` in stack-blitz-tests' tsconfig.spec.json since
  `jasmine-core/lib/jasmine-core/jasmine.js` imported in `test/jasemine-setup.ts`
   is not compatible

Fixes angular#997
@Splaktar Splaktar force-pushed the fix-harness-demos-remove-tslint branch from b901073 to 769a36c Compare July 19, 2021 18:36
@Splaktar Splaktar changed the title fix(stack-blitz-tests): disable Ivy to fix harness examples fix(stack-blitz-tests): revert enabling Ivy as it broke harness examples Jul 19, 2021
Copy link
Collaborator

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@crisbeto crisbeto merged commit e190dc4 into angular:master Jul 29, 2021
@Splaktar Splaktar deleted the fix-harness-demos-remove-tslint branch August 6, 2021 17:42
@Splaktar
Copy link
Contributor Author

Splaktar commented Aug 6, 2021

Issue #1037 tracks properly re-enabling Ivy and ensuring that the Harness Examples work.

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

Successfully merging this pull request may close these issues.

bug(components with test harness example): Missing dependencies in StackBlitz examples
5 participants