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

Replace node polyfills with maintained version #32

Closed
wants to merge 2 commits into from
Closed

Replace node polyfills with maintained version #32

wants to merge 2 commits into from

Conversation

p-m-p
Copy link

@p-m-p p-m-p commented Feb 1, 2023

Possible solution to discussion on #29 @MichaelDeBoey

const path = args.path.replace(prefixFilter, '')
const isCommonjs =
args.namespace !== commonjsNamespace &&
args.kind === 'require-call'
Copy link
Author

Choose a reason for hiding this comment

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

I copied this from the other resolver but didn't really take the time to understand fully what it is doing so may need some consideration.

@remorses
Copy link
Owner

remorses commented Feb 1, 2023

If you want to use this for Remix i think it's better that you create a plugin package in the Remix monorepo so you can quickly fix bugs and add features, i don't want to maintain a core piece of the Remix echosystem

Replacing the polyfill could cause a lot of issues and inconsistencies to other projects already using this plugin

Feel free to copy the plugin source code in the Remix repo

@p-m-p
Copy link
Author

p-m-p commented Feb 1, 2023

If you want to use this for Remix i think it's better that you create a plugin package in the Remix monorepo so you can quickly fix bugs and add features, i don't want to maintain a core piece of the Remix echosystem

Replacing the polyfill could cause a lot of issues and inconsistencies to other projects already using this plugin

Feel free to copy the plugin source code in the Remix repo

Totally agree. I'm not affiliated with Remix just have an outstanding issue there related to this.

I'll leave the patch up if you want to explore it to move away from the deprecated lib or if anyone else wants to take it.

Thanks

@nkrmr
Copy link

nkrmr commented Mar 28, 2023

@p-m-p

There is a PR on Remix to update this plugin:
remix-run/remix#5274

I hope that fixes the issue.

Best regard

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.

4 participants