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 es6 import #232

Merged
merged 4 commits into from
Sep 20, 2018
Merged

Fix es6 import #232

merged 4 commits into from
Sep 20, 2018

Conversation

samtsai
Copy link

@samtsai samtsai commented Sep 20, 2018

I was still having issues with getting the Typings correct with import names, even after #230 , related to #217 #221

Sam Tsai added 4 commits September 19, 2018 14:57
Though no required since it is the default named `index.d.ts` and at root,
it is still advisable.

@see https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html
We need to use the exported `printJS` function.
Previous instructions was slightly misleading as this
can be used as a global library or an exported
module function.
Change interface to use `printJS` both as an import as well
as on window object.
@samtsai
Copy link
Author

samtsai commented Sep 20, 2018

@gabrielmdeal @crabbly I've been having issues in my project getting the typings to work and I think it relates to naming inconsistencies.

@crabbly crabbly merged commit eda321f into crabbly:master Sep 20, 2018
@crabbly
Copy link
Owner

crabbly commented Sep 20, 2018

Hi Sam,
Thank you for your help with the lib! :)

I'll publish these changes to npm soon. Will post an update here once its available there.

@crabbly
Copy link
Owner

crabbly commented Sep 20, 2018

I just realized that this will expose the printJS (window.printJS) variable as a module, overwriting the function.
cc4fa86#diff-11e9f7f953edc64ba14b0cc350ae7b9dR11

@crabbly
Copy link
Owner

crabbly commented Sep 21, 2018

I forgot to mention that this happens when using the build file directly. For ex., in our test.html file.

@crabbly
Copy link
Owner

crabbly commented Sep 21, 2018

Exporting using node's module.export fixes it.
https://github.com/crabbly/Print.js/blob/master/src/index.js

Found explanation here:
https://stackoverflow.com/a/40295288

@samtsai
Copy link
Author

samtsai commented Sep 21, 2018

Thanks @crabbly for the follow up! I'm so glad you found a relevant explanation. I've been consuming libraries more as modules these days and forget the impact on window/global libraries. I'm going to test locally the test.html file as well as in my library.

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

Successfully merging this pull request may close these issues.

2 participants