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

Using the latest Esbuild #108

Merged
merged 6 commits into from
Mar 13, 2024
Merged

Using the latest Esbuild #108

merged 6 commits into from
Mar 13, 2024

Conversation

Janther
Copy link
Collaborator

@Janther Janther commented Jan 24, 2024

pushing past [email protected] has been tricky mainly because the output is automatically wrapped in a function that would add a __esModule: true without us noticing and webpack would see this and automatically try to load parser.default which was undefined so after some detectiving I came up with this solution in src/index.ts.

I also added a browserlist functionality to keep the file size similar to the old esbuild which did not add any Polyfill at all.

@Janther Janther requested a review from fvictorio January 24, 2024 00:29
@Janther Janther force-pushed the esbuild branch 4 times, most recently from 87a9943 to 1da5ec5 Compare January 27, 2024 23:40

export type { ParseOptions } from './types'

export default { ParserError, parse, tokenize, visit }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

after some testing, this was only needed because we were importing a default value in prettier-plugin-solidity

if we change:

import parser from '@solidity-parser/parser';

to:

import { parse as solidityParserParse, visit } from '@solidity-parser/parser';

It works perfectly.
Up to you whether you wanna keep this change or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that default imports won't work anymore?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I checked this locally and it works fine both with CJS and ESM 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

The UMD file also works fine with HTML. I think we can merge this.

@Janther Janther force-pushed the esbuild branch 3 times, most recently from 6a45e53 to 9b78ef0 Compare February 7, 2024 23:17
@Janther Janther force-pushed the esbuild branch 3 times, most recently from 2b7bee8 to e69c6fa Compare February 16, 2024 20:53
@Janther Janther force-pushed the esbuild branch 3 times, most recently from af5fdf4 to 94dec27 Compare February 22, 2024 21:43
@Janther Janther force-pushed the esbuild branch 2 times, most recently from 7920a44 to c5daa60 Compare February 28, 2024 20:14
@Janther Janther force-pushed the esbuild branch 2 times, most recently from 0de491a to c4acb5b Compare March 10, 2024 20:57
@Janther Janther merged commit b965d3b into master Mar 13, 2024
7 checks passed
@Janther Janther deleted the esbuild branch March 13, 2024 11:32
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