feat(rosetta): hoist imports above fixtures #2211
Conversation
When `import` statements are used in a code example, they must be hoisted at the top of the fixture (if any fixture is used), as they otherwise may occur at locations where such statements are illegal (they are only accepted at the top-level scope). Additionaly, added some (hidden) comments to highlight where the hoisted imports begin and end, as well as where the snipped code begins and ends in an attempt to make the "complete source code" a little easier to navigate, especially in the context of larger fixtures.
| @@ -66,7 +66,7 @@ export function completeSource(snippet: TypeScriptSnippet) { | |||
| function parametersFromSourceDirectives( | |||
| source: string, | |||
| ): [string, Record<string, string>] { | |||
| const [firstLine, rest] = source.split('\n', 2); | |||
| const [firstLine, ...rest] = source.split('\n'); | |||
There was a problem hiding this comment.
Note - there was a bug right here that my test highlighted (tests with only 1 line of snippet have missed this entirely).
The culprit being that JavaScript's String.split function doesn't work like most folks expect it to when it comes to the limit: instead of limiting the amount of cuts that will be made in the source (like in java) and having the last slice be "the rest", JavaScript cuts on all separators, and the limit restricts the amount of slices returned.
In other words:
// Example.java
final String abc = "a,b,c";
abc.split(",", 2);
//=> ["a", "b,c"]Versus
// Example.js
const abc = 'a,b,c';
abc.split(',', 2);
//=> ['a', 'b'] // Omitting slice #3, 'c'.
| @@ -0,0 +1,17 @@ | |||
| import { Assembly, SchemaVersion } from '@jsii/spec'; | |||
There was a problem hiding this comment.
This was moved from jsii/assemblies.test.ts, because rosetta.test.ts imports this function; but the importing actually caused all tests in jsii/assemblies.test.ts to be run twice as a collateral.
|
Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it! |
|
Merging (with squash)... |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Merging (with squash)... |
When
importstatements are used in a code example, they must behoisted at the top of the fixture (if any fixture is used), as they
otherwise may occur at locations where such statements are illegal (they
are only accepted at the top-level scope).
Additionaly, added some (hidden) comments to highlight where the hoisted
imports begin and end, as well as where the snipped code begins and ends
in an attempt to make the "complete source code" a little easier to
navigate, especially in the context of larger fixtures.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.