Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(commons): invalid reference to properties.d.ts in dist/types/sdk/reporter/utils.d.ts of allure-js-commons #1162

Merged
merged 6 commits into from
Oct 2, 2024

Conversation

delatrie
Copy link
Collaborator

@delatrie delatrie commented Oct 1, 2024

Context

The PR fixes the invalid reference to properties.d.ts, which doesn't exist in the distribution, causing TypeScript compilation to fail.

The file contains ad-hoc typings for properties, the package we use to read/write environment.properties. *.d.ts files are never included in the emit phase by TypeScript. A reference comment pointing to the file is emitted if the types defined in the file are exported somehow from *.ts files.

In our case, the reason the reference comment gets emitted is because we reexport the entire properties.parse here:

export const parseProperties = properties.parse;

TypeScript infers its type as follows:

export declare const parseProperties: typeof properties.parse;

Which requires access to the type of the original function, hence, the reference.

environment.properties read/write reimplementation

While wrapping properties.parse in an explicitly typed function would work, this PR introduces new functions to stringify/parse environment.properties:

import { stringifyEnvInfo, parseEnvInfo } from "allure-js-commons/sdk/reporter";

The implementation is mainly taken from gagle/node-properties with unneeded features stripped off (sections, namespaces, etc). Other code was adjusted to use them instead of the original ones.

Note

We use parseEnvInfo for testing purposes only; the production code of the integrations only uses stringifyEnvInfo.

Changes in the dependencies

  • properties removed from allure-js-commons

Fixes #1151.

Checklist

@delatrie delatrie added the type:bug Something isn't working label Oct 1, 2024
@github-actions github-actions bot added the theme:api Javascript API related issue label Oct 1, 2024
@delatrie delatrie changed the title fix(commons): invalid reference to properties.d.ts in allure-js-commons/sdk/reporter/utils fix(commons): invalid reference to properties.d.ts in dist/types/sdk/reporter/utils.d.ts of allure-js-commons Oct 1, 2024
@delatrie delatrie marked this pull request as ready for review October 1, 2024 22:01
@delatrie delatrie requested a review from epszaw October 1, 2024 22:01
@delatrie delatrie merged commit f38b0e7 into main Oct 2, 2024
10 checks passed
@delatrie delatrie deleted the properties-typing-fix branch October 2, 2024 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme:api Javascript API related issue type:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrading to latest version, having issue with allure-js-commons dependency
2 participants