Closed stbrody closed 3 years ago
https://github.com/ceramicnetwork/js-ceramic/pull/1164 Fixes the Can't resolve 'logfmt'
error. The new set of errors after that is:
> ceramic-browser@ start /home/spencer/3box/webpack-test
> webpack --mode=production .
assets by status 481 KiB [cached] 1 asset
runtime modules 1.02 KiB 6 modules
orphan modules 3.97 KiB [orphan] 2 modules
modules by path ./node_modules/ 1.09 MiB
javascript modules 1.09 MiB 186 modules
json modules 3.9 KiB
./node_modules/@ceramicnetwork/http-client/package.json 2.1 KiB [built] [code generated]
./node_modules/elliptic/package.json 1.8 KiB [built] [code generated]
optional modules 30 bytes [optional]
crypto (ignored) 15 bytes [optional] [built] [code generated]
buffer (ignored) 15 bytes [optional] [built] [code generated]
./index.js 39 bytes [built] [code generated]
ERROR in ./node_modules/@ceramicnetwork/common/lib/logger-provider.js 8:31-46
Module not found: Error: Can't resolve 'path' in '/home/spencer/3box/webpack-test/node_modules/@ceramicnetwork/common/lib'
BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.
If you want to include a polyfill, you need to:
- add a fallback 'resolve.fallback: { "path": require.resolve("path-browserify") }'
- install 'path-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
resolve.fallback: { "path": false }
@ ./node_modules/@ceramicnetwork/common/lib/index.js 19:13-41
@ ./node_modules/@ceramicnetwork/http-client/lib/ceramic-http-client.js 10:17-50
@ ./index.js 1:0-38
ERROR in ./node_modules/@ceramicnetwork/common/lib/logger-provider.js 9:29-42
Module not found: Error: Can't resolve 'os' in '/home/spencer/3box/webpack-test/node_modules/@ceramicnetwork/common/lib'
BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.
If you want to include a polyfill, you need to:
- add a fallback 'resolve.fallback: { "os": require.resolve("os-browserify/browser") }'
- install 'os-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
resolve.fallback: { "os": false }
@ ./node_modules/@ceramicnetwork/common/lib/index.js 19:13-41
@ ./node_modules/@ceramicnetwork/http-client/lib/ceramic-http-client.js 10:17-50
@ ./index.js 1:0-38
ERROR in ./node_modules/@overnightjs/logger/lib/Logger.js 4:9-22
Module not found: Error: Can't resolve 'fs' in '/home/spencer/3box/webpack-test/node_modules/@overnightjs/logger/lib'
@ ./node_modules/@overnightjs/logger/lib/index.js 4:21-40
@ ./node_modules/@ceramicnetwork/common/lib/loggers.js 26:17-47
@ ./node_modules/@ceramicnetwork/common/lib/index.js 21:13-33
@ ./node_modules/@ceramicnetwork/http-client/lib/ceramic-http-client.js 10:17-50
@ ./index.js 1:0-38
ERROR in ./node_modules/@overnightjs/logger/lib/Logger.js 5:9-22
Module not found: Error: Can't resolve 'os' in '/home/spencer/3box/webpack-test/node_modules/@overnightjs/logger/lib'
BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.
If you want to include a polyfill, you need to:
- add a fallback 'resolve.fallback: { "os": require.resolve("os-browserify/browser") }'
- install 'os-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
resolve.fallback: { "os": false }
@ ./node_modules/@overnightjs/logger/lib/index.js 4:21-40
@ ./node_modules/@ceramicnetwork/common/lib/loggers.js 26:17-47
@ ./node_modules/@ceramicnetwork/common/lib/index.js 21:13-33
@ ./node_modules/@ceramicnetwork/http-client/lib/ceramic-http-client.js 10:17-50
@ ./index.js 1:0-38
ERROR in ./node_modules/@overnightjs/logger/lib/Logger.js 6:11-26
Module not found: Error: Can't resolve 'path' in '/home/spencer/3box/webpack-test/node_modules/@overnightjs/logger/lib'
BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.
If you want to include a polyfill, you need to:
- add a fallback 'resolve.fallback: { "path": require.resolve("path-browserify") }'
- install 'path-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
resolve.fallback: { "path": false }
@ ./node_modules/@overnightjs/logger/lib/index.js 4:21-40
@ ./node_modules/@ceramicnetwork/common/lib/loggers.js 26:17-47
@ ./node_modules/@ceramicnetwork/common/lib/index.js 21:13-33
@ ./node_modules/@ceramicnetwork/http-client/lib/ceramic-http-client.js 10:17-50
@ ./index.js 1:0-38
ERROR in ./node_modules/@overnightjs/logger/lib/index.js 3:14-30
Module not found: Error: Can't resolve 'tslib' in '/home/spencer/3box/webpack-test/node_modules/@overnightjs/logger/lib'
@ ./node_modules/@ceramicnetwork/common/lib/loggers.js 26:17-47
@ ./node_modules/@ceramicnetwork/common/lib/index.js 21:13-33
@ ./node_modules/@ceramicnetwork/http-client/lib/ceramic-http-client.js 10:17-50
@ ./index.js 1:0-38
ERROR in ./node_modules/@stablelib/random/lib/source/node.js 11:29-46
Module not found: Error: Can't resolve 'crypto' in '/home/spencer/3box/webpack-test/node_modules/@stablelib/random/lib/source'
BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.
If you want to include a polyfill, you need to:
- add a fallback 'resolve.fallback: { "crypto": require.resolve("crypto-browserify") }'
- install 'crypto-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
resolve.fallback: { "crypto": false }
@ ./node_modules/@stablelib/random/lib/source/system.js 6:13-30
@ ./node_modules/@stablelib/random/lib/random.js 5:15-41
@ ./node_modules/@ceramicnetwork/doctype-tile/lib/tile-doctype.js 35:17-45
@ ./node_modules/@ceramicnetwork/doctype-tile/lib/index.js 13:13-38
@ ./node_modules/@ceramicnetwork/http-client/lib/ceramic-http-client.js 11:23-62
@ ./index.js 1:0-38
ERROR in ./node_modules/colors/lib/system/supports-colors.js 28:9-22
Module not found: Error: Can't resolve 'os' in '/home/spencer/3box/webpack-test/node_modules/colors/lib/system'
BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.
If you want to include a polyfill, you need to:
- add a fallback 'resolve.fallback: { "os": require.resolve("os-browserify/browser") }'
- install 'os-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
resolve.fallback: { "os": false }
@ ./node_modules/colors/lib/colors.js 41:23-72
@ ./node_modules/colors/lib/index.js 1:13-32
@ ./node_modules/@overnightjs/logger/lib/Logger.js 3:13-30
@ ./node_modules/@overnightjs/logger/lib/index.js 4:21-40
@ ./node_modules/@ceramicnetwork/common/lib/loggers.js 26:17-47
@ ./node_modules/@ceramicnetwork/common/lib/index.js 21:13-33
@ ./node_modules/@ceramicnetwork/http-client/lib/ceramic-http-client.js 10:17-50
@ ./index.js 1:0-38
ERROR in ./node_modules/logfmt/lib/body_parser_stream.js 3:15-41
Module not found: Error: Can't resolve 'stream' in '/home/spencer/3box/webpack-test/node_modules/logfmt/lib'
BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.
If you want to include a polyfill, you need to:
- add a fallback 'resolve.fallback: { "stream": require.resolve("stream-browserify") }'
- install 'stream-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
resolve.fallback: { "stream": false }
@ ./node_modules/logfmt/logfmt.js 8:23-58
@ ./node_modules/@ceramicnetwork/common/lib/loggers.js 27:28-45
@ ./node_modules/@ceramicnetwork/common/lib/index.js 21:13-33
@ ./node_modules/@ceramicnetwork/http-client/lib/ceramic-http-client.js 10:17-50
@ ./index.js 1:0-38
ERROR in ./node_modules/logfmt/lib/streaming.js 3:18-47
Module not found: Error: Can't resolve 'stream' in '/home/spencer/3box/webpack-test/node_modules/logfmt/lib'
BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.
If you want to include a polyfill, you need to:
- add a fallback 'resolve.fallback: { "stream": require.resolve("stream-browserify") }'
- install 'stream-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
resolve.fallback: { "stream": false }
@ ./node_modules/logfmt/logfmt.js 6:23-49
@ ./node_modules/@ceramicnetwork/common/lib/loggers.js 27:28-45
@ ./node_modules/@ceramicnetwork/common/lib/index.js 21:13-33
@ ./node_modules/@ceramicnetwork/http-client/lib/ceramic-http-client.js 10:17-50
@ ./index.js 1:0-38
ERROR in ./node_modules/through/index.js 1:13-30
Module not found: Error: Can't resolve 'stream' in '/home/spencer/3box/webpack-test/node_modules/through'
BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.
If you want to include a polyfill, you need to:
- add a fallback 'resolve.fallback: { "stream": require.resolve("stream-browserify") }'
- install 'stream-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
resolve.fallback: { "stream": false }
@ ./node_modules/logfmt/lib/streaming.js 2:18-36
@ ./node_modules/logfmt/logfmt.js 6:23-49
@ ./node_modules/@ceramicnetwork/common/lib/loggers.js 27:28-45
@ ./node_modules/@ceramicnetwork/common/lib/index.js 21:13-33
@ ./node_modules/@ceramicnetwork/http-client/lib/ceramic-http-client.js 10:17-50
@ ./index.js 1:0-38
11 errors have detailed information that is not shown.
Use 'stats.errorDetails: true' resp. '--stats-error-details' to show it.
webpack 5.28.0 compiled with 11 errors in 4769 ms
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! ceramic-browser@ start: `webpack --mode=production .`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the ceramic-browser@ start script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm ERR! A complete log of this run can be found in:
npm ERR! /home/spencer/.npm/_logs/2021-03-26T18_19_21_234Z-debug.log
To summarize:
Dependencies (and where that dependency originates) that seem to have browser-based polyfills we could provide:
Dependencies that don't seem to have obvious browser polyfills:
So it seems like most of these we could clean up on our side, but only if we get rid of our use of overnightjs
. If we want to keep using overnightjs
, then it seems like we can provide browser-based polyfills for just about everything except for tslib
. I'm not sure what we can do about overnightjs
using tslib
.
@oed @ukstv @PaulLeCam @valmack @zachferland
After going full circle on this bundler/target thing, im not seeing anything that differs from prior releases, at least for http client (was the issue just reported with webpack5?). There is issues with core running in browser now, mostly just the logger and single fs call. The path forward to fix that and to get webpack5 to work more nicely by default would be to have a logger-provider.ts and logger-provider-browser.ts (seems like a bit of refactoring there), then in package.json have the browser target reference that
"browser": {
"./lib/logger-provider.js": "./lib/logger-provider-browser.js,
},
The leveldb/fs should be fairly simple to solve as well.
Lastly also for webpack5 (http-client), @stablelib/random needs small package.json change if we want webpack5 to work by default, can submit that, or since just the random module, may be another way around by replacing. May be other packages that have issues in core though, did try build that directly yet.
To sum up, seems like we should be releasing, bundlers that polyfill by default (webpack4) will work still, ones that dont, devs will likely have enough awareness to get it to work. Then the following iterations:
1) http client work by default with webpack5 (stablelib change, export empty logger for browser target and/or refactor) 2) core work in browser and build with webpack5 by default (changes above) 3) limit regressions, part of build/test pipeline, would likely just be use bundler, and running against some unit test in headless browser
We can decide how want to prioritize
I may be missing something about the originally reported issue though, and just to confirm the logger is not used in the http-client right?
cc @stbrody @oed
Getting the logger out id definitely the top prio since it's actually blocking things running in browser. This means that people could tell webpack5 to ignore the errors.
Second thing to do would be to fix these errors as well.
yeah but http-client doesnt use logger right? and core has another issue besides that with leveldb/fs anyways, so that alone wont work
so http-client doesnt seem blocked for browser, while core is logger and leveldb/fs (maybe other things)
and how of prio is running core in browser?
Prio is for http-client to run in browser! Core running in browser is a nice to have.
So it sounds like the response to all the people who have been reporting this in discord is that they need to either use webpack4 or provide their own polyfills, but our belief is that if they can get the bundler to work, everything will actually already work at runtime?
The path forward to fix that and to get webpack5 to work more nicely by default would be to have a logger-provider.ts and logger-provider-browser.ts
This doesn't seem very hard to do, maybe a day of work, and might save us a lot of headache
To me the root cause of the problem is presence of LoggerProvider in common. And the single reason for this is Context
depends on it. Seems, like removing LoggerProvider from Context and moving it to logger
package should solve the issue.
Why LoggerProvider exists in common @stbrody ?
Why LoggerProvider exists in common @stbrody ?
We want the logger to be available as widely as possible. Any part of the codebase should be able to log if it needs to. But wanting to log in some piece of code shouldn't prevent that code from being usable in-browser.
We want the logger to be available as widely as possible. Any part of the codebase should be able to log if it needs to. But wanting to log in some piece of code shouldn't prevent that code from being usable in-browser.
Yes that's totally achievable but that requires being more strict with the architecture:
common
, utils
, types
or similar generic package shouldn't have dependency on other packages in the same monorepo (ex client
can import common
but not the other way around as it would create a circular dependency).common-node
and common-browser
) or be explicitly handled (see https://webpack.js.org/guides/package-exports/#target-environment and https://webpack.js.org/guides/package-exports/#providing-different-versions-depending-on-target-environment, used in example below).docid
package, rather than being in common
. It could be the same for the logger.common
? For such cases we can have the following:common/index.ts
export type LogLevel = 'error' | 'warn' | 'info' | 'debug' | ...
export interface Logger {
log(level: LogLevel, message: string, data?: any): void
}
// Can be safely used by any package without having to import the logger package
export type CeramicConfig = {
logger?: Logger
}
logger/console.ts
import type { Logger } from 'common'
export class ConsoleLogger implements Logger {
log(level, message) {
console.log(`[${level}] ${message}`)
}
}
logger/fs.ts
import type { Logger } from 'common'
import { writeFile } from 'fs' // OK to import node modules here
export class FileLogger implements Logger {
log(level, message) {
// OK to call writeFile() here
}
}
logger/index.ts
// Only import loggers that work on any platform here
import { ConsoleLogger } from './console'
export { ConsoleLogger } from './console'
// The default logger will use the console implementation
export const DefaultLogger = ConsoleLogger
logger/node.ts
// We can import node-specific modules here
import { FileLogger } from './fs'
export { ConsoleLogger } from './console'
export { FileLogger } from './fs'
// The default logger will use the node-specific implementation
export const DefaultLogger = FileLogger
logger/package.json
{
...
"exports": {
"node": "./dist/node.js",
"default": "./dist/index.js"
}
}
client/index.ts
import type { CeramicConfig } from 'common'
import { DefaultLogger } from 'logger' // will import the environment-specific logger
export class Client {
constructor(config: CeramicConfig = {}) {
// Don't need to worry about platform-specific concerns here
this.logger = config.logger ?? new DefaultLogger()
}
}
This is just example code I wrote in this comment and not something I actually tried to run so it might not work as-is, but hopefully that covers the idea of the logical separation of packages and targets.
Simple test of building http-client with webpack found the following errors: