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.

Sign up for GitHub

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

Jump to bottom

fix(node-modules-polyfill): replace rollup-plugin-node-polyfills with rollup-plugin-polyfill-node #19

Merged
merged 1 commit into from
Jan 26, 2023
Merged

fix(node-modules-polyfill): replace rollup-plugin-node-polyfills with rollup-plugin-polyfill-node #19

merged 1 commit into from
Jan 26, 2023

Conversation

MichaelDeBoey
Copy link
Contributor

@MichaelDeBoey MichaelDeBoey commented Jun 29, 2022 •
edited
Loading

Closes #18

@MichaelDeBoey MichaelDeBoey changed the title fix: replace rollup-plugin-node-polyfills with rollup-plugin-polyfill-node fix(node-modules-polyfill): replace rollup-plugin-node-polyfills with rollup-plugin-polyfill-node Jun 29, 2022
@MichaelDeBoey MichaelDeBoey mentioned this pull request Jun 29, 2022
Closed
@MichaelDeBoey MichaelDeBoey force-pushed the replace-node-polyfills-with-maintained-version branch from 24c440f to d9a09c5 Compare June 29, 2022 16:01
@MichaelDeBoey MichaelDeBoey mentioned this pull request Jun 29, 2022
Merged
@MichaelDeBoey MichaelDeBoey force-pushed the replace-node-polyfills-with-maintained-version branch from d9a09c5 to e9fbfc8 Compare July 6, 2022 17:15
@MichaelDeBoey MichaelDeBoey marked this pull request as ready for review July 6, 2022 17:15
@MichaelDeBoey
Copy link
Contributor Author

@remorses This one's good to be merged now I think

@imranbarbhuiya
Copy link
Contributor

imranbarbhuiya commented Jul 10, 2022 •
edited
Loading

using this pr (after building locally), I get this error
image

Note
used #20 to support node: protocol. But without that pr and without node: it still throws

@MichaelDeBoey
Copy link
Contributor Author

MichaelDeBoey commented Jul 19, 2022 •
edited
Loading

@imranbarbhuiya That problem has nothing to do with the changes in this PR

@remorses Is there anything I still need to do to get this one merged?

@MichaelDeBoey MichaelDeBoey force-pushed the replace-node-polyfills-with-maintained-version branch from e9fbfc8 to 819d287 Compare July 19, 2022 19:34
@remorses
Copy link
Owner

I have to test everything works locally first, I have been busy

@MichaelDeBoey
Copy link
Contributor Author

@remorses Let me know if I can help you out in some way

@matikucharski
Copy link

@remorses When it can be merged?

@MichaelDeBoey
Copy link
Contributor Author

MichaelDeBoey commented Sep 17, 2022 •
edited
Loading

@remorses Any update on this one?
Or anything I need to update?

We would love to have this merged for the Remix cli

@p-m-p p-m-p mentioned this pull request Oct 1, 2022
Closed
This was referenced Dec 14, 2022
Closed
Open
Open
@MichaelDeBoey MichaelDeBoey force-pushed the replace-node-polyfills-with-maintained-version branch 2 times, most recently from 648cc94 to e3e6f79 Compare December 14, 2022 11:52
@MichaelDeBoey
Copy link
Contributor Author

@connorjclark @imranbarbhuiya I updated the PR to account for the actual content instead of the path
Could you please check if everything is working as expected now?

@MichaelDeBoey MichaelDeBoey requested review from giuseppeg and connorjclark and removed request for connorjclark and giuseppeg December 14, 2022 12:27
@MichaelDeBoey MichaelDeBoey mentioned this pull request Dec 14, 2022
Merged
@sxzz sxzz mentioned this pull request Dec 17, 2022
Closed
@ice1000
Copy link

ice1000 commented Dec 17, 2022

💯💯💯💯 nice!!!

@MichaelDeBoey
Copy link
Contributor Author

@remorses I think this one can be merged now
Anything else I need to do to get this one over the line?

@i7eo
Copy link

i7eo commented Jan 23, 2023

Is there progress?

@MichaelDeBoey
Copy link
Contributor Author

@i7eo I'm a bit waiting for @remorses for this to get merged now, so we need to have some patience I'm afraid

@MichaelDeBoey MichaelDeBoey force-pushed the replace-node-polyfills-with-maintained-version branch from e3e6f79 to c313c5c Compare January 23, 2023 18:29
@MichaelDeBoey MichaelDeBoey mentioned this pull request Jan 23, 2023
Merged
@MichaelDeBoey MichaelDeBoey force-pushed the replace-node-polyfills-with-maintained-version branch from c313c5c to 2fe0172 Compare January 24, 2023 00:21
@i7eo
Copy link

i7eo commented Jan 24, 2023

@i7eo I'm a bit waiting for @remorses for this to get merged now, so we need to have some patience I'm afraid

alright, I got.

@remorses remorses merged commit e390d6f into remorses:master Jan 26, 2023
@ice1000
Copy link

ice1000 commented Jan 26, 2023

Thanks very much!!!

@MichaelDeBoey MichaelDeBoey deleted the replace-node-polyfills-with-maintained-version branch January 26, 2023 17:25
@MichaelDeBoey
Copy link
Contributor Author

MichaelDeBoey commented Jan 26, 2023 •
edited
Loading

@remorses It appears we have the same error @connorjclark was talking about in #19 (comment) in the Remix repo now that we updated to v0.2.0
https://github.com/remix-run/remix/actions/runs/4017689920/jobs/6902360260#step:7:122

Cannot read directory "\x00polyfill-node._stream_duplex": invalid argument

Does any of you know a possible solution for that? 🙏

This was referenced Jan 26, 2023
Closed
Closed
@MichaelDeBoey MichaelDeBoey mentioned this pull request Aug 24, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@giuseppeg giuseppeg giuseppeg left review comments

@connorjclark connorjclark Awaiting requested review from connorjclark

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[node-modules-polyfill] Replace unmaintained rollup-plugin-node-polyfills dependency
8 participants
@MichaelDeBoey @imranbarbhuiya @remorses @matikucharski @ice1000 @i7eo @giuseppeg @connorjclark