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: update dependencies and fix types for Proxy #66

Merged
merged 5 commits into from
Sep 8, 2021

Conversation

nolanlawson
Copy link
Contributor

Updates dependencies, most notably TypeScript which has a new definition for Proxy objects which breaks some of our existing types: microsoft/TypeScript#35594

Notably it looks like we've been assuming that PropertyKey (which is string | number | symbol) is fine to be used as proxy property keys, when actually the spec only allows string | symbol. So I added a ProxyPropertyKey type to handle this.

@@ -40,7 +40,8 @@ function rollupConfig({ formats, prod }) {
format,
banner,
footer,
file: path.join(targetDirectory, targetName)
file: path.join(targetDirectory, targetName),
exports: 'auto'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does not seem to change the output file format, and suppresses a warning from Rollup.

Copy link
Member

@pmdartus pmdartus left a comment

Choose a reason for hiding this comment

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

Minor nitpicks otherwise LGTM.

yarn.lock Outdated
@@ -9,11 +9,23 @@
dependencies:
"@babel/highlight" "^7.12.13"

"@babel/code-frame@^7.14.5":
Copy link
Member

Choose a reason for hiding this comment

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

I would recommend running yarn upgrade to get rid of the duplicate packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

rollup.config.js Outdated Show resolved Hide resolved
rollup.config.js Outdated Show resolved Hide resolved
nolanlawson and others added 4 commits September 8, 2021 15:19
Co-authored-by: Pierre-Marie Dartus <p.dartus@salesforce.com>
Co-authored-by: Pierre-Marie Dartus <p.dartus@salesforce.com>
@nolanlawson nolanlawson changed the title chore: update dependencies fix: update dependencies and fix types for Proxy Sep 8, 2021
@nolanlawson nolanlawson merged commit fb7daec into master Sep 8, 2021
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.

None yet

3 participants