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

Provide default export from the main module #860

Merged
merged 2 commits into from Jul 16, 2021

Conversation

ilammy
Copy link
Collaborator

@ilammy ilammy commented Jul 14, 2021

Because WasmThemis 0.13.X has been using CommonJS modules, various transpilers interpreted

import themis from 'wasm-themis';

as a valid way to import WasmThemis stuff under themis namespace in genuine ES6 code. (For example, WasmThemis example with webpack is written like that.)

The "correct" way to do this is as follows:

import * as themis from 'wasm-themis';

but we need to make sure that the previous form still works.

Exporting things twice is awkward, but when did “it looks awful” was a valid argument against making a breaking change?

(While we're here, drop unnecessary secure_cell.ts file. index.ts exports all Secure Cell variations individually and this file is not really used anywhere else. File structure of WasmThemis is implementation detail (except for libthemis.wasm location, I guess). You're not supposed to peek into it.)

Checklist

  • Change is covered by automated tests
  • The coding guidelines are followed
  • Example projects and code samples are up-to-date (they already use this form)
  • Changelog is updated (should not have changed in the first place)

Because WasmThemis 0.13.X has been using CommonJS modules, various
transpilers interepreted

    import themis from 'wasm-themis';

as a valid way to import WasmThemis stuff under "themis" namespace
in genuine ES6 code.

The "correct" way to do this is as follows:

    import * as themis from 'wasm-themis';

but we need to make sure that the previous form still works.

It's awkward, but when did "it looks awful" was an arguments against
making a breaking change?
"index.ts" exports all Secure Cell variations individually and this file
is not really used anywhere else. File structure of WasmThemis is
implementation detail. You're not supposed to peek into it.
@ilammy ilammy added the W-WasmThemis 🌐 Wrapper: WasmThemis, JavaScript API, npm packages label Jul 14, 2021
@ilammy ilammy requested review from vixentael and a team July 14, 2021 15:06
Copy link
Collaborator

@Lagovas Lagovas left a comment

Choose a reason for hiding this comment

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

lgtm


// And of course export all public things individually.

export {
Copy link
Collaborator

Choose a reason for hiding this comment

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

wat?) I need to go on some nodejs/es6 courses and learn how works new imports in javascript world....

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's not like I have much clue myself, lol, I'm just reading the MDN:

Copy link
Contributor

@vixentael vixentael left a comment

Choose a reason for hiding this comment

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

👏

Comment on lines -19 to +22
export { SecureCellSeal } from "./secure_cell_seal";
export { SecureCellTokenProtect } from "./secure_cell_token_protect";
export { SecureCellContextImprint } from "./secure_cell_context_imprint";
export { ThemisError, ThemisErrorCode } from "./themis_error";
export {
import { SecureCellSeal } from "./secure_cell_seal";
import { SecureCellTokenProtect } from "./secure_cell_token_protect";
import { SecureCellContextImprint } from "./secure_cell_context_imprint";
import { ThemisError, ThemisErrorCode } from "./themis_error";
import {
Copy link
Contributor

Choose a reason for hiding this comment

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

unexpected :)

@ilammy ilammy merged commit 94f22d2 into cossacklabs:wasm-typescript Jul 16, 2021
@ilammy ilammy deleted the wasm-default-export branch July 16, 2021 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
W-WasmThemis 🌐 Wrapper: WasmThemis, JavaScript API, npm packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants