Skip to content

[ts] enable support for iterators in browsers#22986

Merged
spalger merged 3 commits intoelastic:masterfrom
spalger:implement/ts/browser-iterator-support
Sep 13, 2018
Merged

[ts] enable support for iterators in browsers#22986
spalger merged 3 commits intoelastic:masterfrom
spalger:implement/ts/browser-iterator-support

Conversation

@spalger
Copy link
Copy Markdown
Contributor

@spalger spalger commented Sep 13, 2018

I was surprised when I tried to spread a Set in TypeScript and the browser complained about Set.slice() not being defined. This is because TypeScript does not automatically enable support for iterators when targeting earlier ES versions, like we do in the browser, unless you use the "downlevelIteration": true compiler option. This injects some helpers into the necessary files for reading/spreading iterators, which can be stuffed behind an import statement with using the "importHelpers": true compiler option and include tslib in our dependencies. This is already a dependency of several of our packages, so it shouldn't cause any additional modules.

@spalger spalger added review Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v7.0.0 v6.5.0 labels Sep 13, 2018
@spalger spalger requested a review from epixa September 13, 2018 01:47
@spalger spalger force-pushed the implement/ts/browser-iterator-support branch from 8f60c3b to be123de Compare September 13, 2018 01:48
@elasticmachine

This comment has been minimized.

@spalger
Copy link
Copy Markdown
Contributor Author

spalger commented Sep 13, 2018

00:04:27.831 ERROR: 'yarn kbn bootstrap' caused changes to the following files:
00:04:27.832
00:04:27.832 packages/kbn-es/yarn.lock
00:04:27.832 packages/kbn-i18n/yarn.lock
00:04:27.832 packages/kbn-test/yarn.lock

Weird, when I ran yarn kbn bootstrap locally and those packages reported "Already up-to-date", yarn.lock file wasn't updated until I deleted the .yarn-integrity files... ugh

@elasticmachine

This comment has been minimized.

@spalger spalger removed the request for review from epixa September 13, 2018 17:59
@spalger spalger force-pushed the implement/ts/browser-iterator-support branch from 5107716 to ea64458 Compare September 13, 2018 18:00
@spalger
Copy link
Copy Markdown
Contributor Author

spalger commented Sep 13, 2018

00:14:19.256     Timeout - Async callback was not invoked within the 5000ms timeout specified by jest.setTimeout.
00:14:19.256 
00:14:19.267       37 |   const buildFiles = PLUGIN.buildSourcePatterns;
00:14:19.276       38 | 
00:14:19.282     > 39 |   it('removes development properties from package.json', async () => {
00:14:19.285          |   ^
00:14:19.286       40 |     expect(PLUGIN.pkg.scripts).not.toBeUndefined();
00:14:19.289       41 |     expect(PLUGIN.pkg.devDependencies).not.toBeUndefined();
00:14:19.291       42 | 

Similar tests in other PRs have failed so I created #23005 to move these tests to the jest integration suite.

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@spalger spalger requested a review from epixa September 13, 2018 20:21
"topojson-client": "3.0.0",
"trunc-html": "1.0.2",
"trunc-text": "1.0.2",
"tslib": "^1.9.3",
Copy link
Copy Markdown
Member

@tylersmalley tylersmalley Sep 13, 2018

Choose a reason for hiding this comment

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

Is this supposed to be a devDependency? We only use the typescript runtime during development, correct?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nope, TypeScript will inject require statements for tslib when we use things like a for...of over an iterable, so tslib needs to be installed outside of dev too.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, gotcha. Thanks.

@spalger spalger merged commit b55705e into elastic:master Sep 13, 2018
@spalger spalger added the v6.4.1 label Sep 13, 2018
spalger pushed a commit to spalger/kibana that referenced this pull request Sep 13, 2018
I was surprised when I tried to spread a `Set` in TypeScript and the browser complained about `Set.slice()` not being defined. This is because TypeScript does not automatically enable support for iterators when targeting earlier ES versions, like we do in the browser, unless you use the `"downlevelIteration": true` compiler option. This injects some helpers into the necessary files for reading/spreading iterators, which can be stuffed behind an import statement with using the `"importHelpers": true` compiler option and include `tslib` in our dependencies. This is already a dependency of several of our packages, so it shouldn't cause any additional modules.
spalger pushed a commit to spalger/kibana that referenced this pull request Sep 13, 2018
I was surprised when I tried to spread a `Set` in TypeScript and the browser complained about `Set.slice()` not being defined. This is because TypeScript does not automatically enable support for iterators when targeting earlier ES versions, like we do in the browser, unless you use the `"downlevelIteration": true` compiler option. This injects some helpers into the necessary files for reading/spreading iterators, which can be stuffed behind an import statement with using the `"importHelpers": true` compiler option and include `tslib` in our dependencies. This is already a dependency of several of our packages, so it shouldn't cause any additional modules.
spalger pushed a commit that referenced this pull request Sep 13, 2018
Backports the following commits to 6.x:
 - [ts] enable support for iterators in browsers  (#22986)
@spalger
Copy link
Copy Markdown
Contributor Author

spalger commented Sep 13, 2018

6.5/6.x: 489779a
6.4.2: 73c8166

@spalger spalger deleted the implement/ts/browser-iterator-support branch September 14, 2018 20:47
spalger pushed a commit that referenced this pull request Sep 21, 2018
I was surprised when I tried to spread a `Set` in TypeScript and the browser complained about `Set.slice()` not being defined. This is because TypeScript does not automatically enable support for iterators when targeting earlier ES versions, like we do in the browser, unless you use the `"downlevelIteration": true` compiler option. This injects some helpers into the necessary files for reading/spreading iterators, which can be stuffed behind an import statement with using the `"importHelpers": true` compiler option and include `tslib` in our dependencies. This is already a dependency of several of our packages, so it shouldn't cause any additional modules.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport pending review Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v6.4.2 v6.5.0 v7.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants