Bug 1693516 - Publish type declarations along with @mozilla/glean package #85
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 8 commits
February 23, 2021 13:23
It is important at least for now have these two samples. In the future we should have integrations tests to verify that these are working.
This is in preparation for the next commit. The issue is: Typescript will not resolve type aliases when compiling code (see: microsoft/TypeScript#26722 (comment)). Webpack will do that, but you'll that in the next commit I move away from using webpack for compiling the library. I personaly hate having the `../../../` everywhere but unless we are willing to write our own tool that resolves that on the compiled code, I don't see what else we could do.
- Why stop bundling? It makes it easier to provide the type declarations. - But, how? If we bundle our code, we need to create type declarations on top of the bundled code, but when the code is already bundled we lose all the file paths and type richness (bundled code is Javascript after all), which means, in order to get a rich declaration file we would need to write it ourselves. That would be kinda stupid work, since we are using typescript and have all the types information. - Ok well, create the declaration files based on the unbundled code Yes sure, that is the obvious solution and is also what this commit prepares for. Using declaration files from unbundled code on top of bundled code, does not work, so we need to provide the code in an unbundled format in order for the declaration paths to be correct. - Observations The reflex to use webpack and bundle + minify our code, came from my web development days. I looked at multiple libraries and it is common place to provide the library code unbundled. Developers will consume that library and then bundle it themselves with their own build system. When we decide to target static websites, we can provide a bundled version of our code using a CDN, but that is IMO a bridge we can cross when we get there. We already also have a bug (https://bugzilla.mozilla.org/show_bug.cgi?id=1693039) to provide conditional exports for users coming from different environments, but since our first consumers will be bergamot and rally and both are ok with ES6 modules, we are fine for now.
Sadly, Qt _needs_ the bundled code, so we provide a special build for it.
It was necessary to move away from requiring the package json. Because we were requiring it, it was showing up as one of the compiled files and that confuses consumers that are importing the @mozilla/glean package, because they will resolve the first found package.json.
This probably got lost on one of the previous commits.
Dexterp37
approved these changes
Feb 23, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For reference, an npm publish dry run looks like:
Also, glean_parser changes are underway. I have been working on those concurrently with this PR.