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

Use require() for dynamic imports, use new name of solid-auth-fetcher #52

Merged
merged 3 commits into from
May 11, 2020

Conversation

Vinnl
Copy link
Contributor

@Vinnl Vinnl commented May 7, 2020

Webpack issues a warning when trying to import() a module that is
not present, whereas it does not do so — or at least, not in a way
that blocks Next.js from building the app — with require(). Since
we're requiring a bundler anyway (since we're using npm package
names rather than import paths), we're not any worse off using a
non-standard module system for these.

For more info, see: webpack/webpack#7713

I still couldn't get solid-auth-fetcher to work, but this should at least make it work with solid-auth-client and native fetch in Next.js, which the Pod Manager will likely use.

@Vinnl Vinnl requested a review from NSeydoux May 7, 2020 13:54
@Vinnl Vinnl self-assigned this May 7, 2020
It is now published under the @inrupt scope.
@Vinnl Vinnl force-pushed the fix/dynamic-imports branch 3 times, most recently from bd6e643 to 60c5e0a Compare May 8, 2020 07:47
@Vinnl Vinnl marked this pull request as ready for review May 8, 2020 07:49
Webpack issues a warning when trying to import() a module that is
not present, whereas it does not do so — or at least, not in a way
that blocks Next.js from building the app — with require(). Since
we're requiring a bundler anyway (since we're using npm package
names rather than import paths), we're not any worse off using a
non-standard module system for these.

For more info, see: webpack/webpack#7713
@Vinnl Vinnl merged commit c25e133 into master May 11, 2020
@Vinnl Vinnl deleted the fix/dynamic-imports branch May 11, 2020 09:49
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

2 participants