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

Extract Logger into its own package #2165

Merged
merged 9 commits into from
Oct 18, 2018

Conversation

webtaculars
Copy link
Contributor

↪️ Pull Request

Extract logger code into a separate package inside the monorepo

closes [#2134]

💻 Examples

🚨 Test instructions

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@devongovett devongovett changed the base branch from workerfarm-package to master October 18, 2018 02:47
@devongovett devongovett changed the base branch from master to workerfarm-package October 18, 2018 02:47
@@ -6,7 +6,7 @@ const md5 = require('./utils/md5');
const isURL = require('./utils/is-url');
const config = require('./utils/config');
const syncPromise = require('./utils/syncPromise');
const logger = require('./Logger');
const logger = require('@parcel/Logger');
Copy link
Member

Choose a reason for hiding this comment

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

should be lower case: @parcel/logger

"name": "babel-plugin-autoinstall"
"name": "babel-plugin-autoinstall",
"devDependencies": {
"@babel/core": "^7.1.2"
Copy link
Member

Choose a reason for hiding this comment

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

this shouldn't change - part of the test is asserting that this is added.

@devongovett devongovett changed the base branch from workerfarm-package to master October 18, 2018 02:49
@devongovett
Copy link
Member

Awesome, thanks! Can you rebase with master?

@devongovett
Copy link
Member

I removed the original emoji and prettyError files from utils and exposed them as part of the @parcel/logger package instead.

@interglobalmedia
Copy link

I have parcel-bundler 1.11.0 installed locally (don't want to install globally) and using npx as a result. Everything was fine in development, but can't build my assets for production. Am getting the following in Terminal console:

npm run build                                                      ⏎ ✖ ✹

> [email protected] build /Users/mariacam/Development/html5-drag-and-drop
> npx parcel build index.js -d dist

⚠️  Cannot find module 'parcel-bundler/src/Logger'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:603:15)
    at Function.Module._load (internal/modules/cjs/loader.js:529:25)
    at Module.require (internal/modules/cjs/loader.js:658:17)
    at require (/Users/mariacam/Development/html5-drag-and-drop/node_modules/v8-compile-cache/v8-compile-cache.js:159:20)
    at Object.<anonymous> (/Users/mariacam/Development/html5-drag-and-drop/node_modules/parcel-plugin-eslint/index.js:1:78)
    at Module._compile (/Users/mariacam/Development/html5-drag-and-drop/node_modules/v8-compile-cache/v8-compile-cache.js:178:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:733:10)
    at Module.load (internal/modules/cjs/loader.js:620:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:560:12)
    at Function.Module._load (internal/modules/cjs/loader.js:552:3)
🚨  No entries found.
    at Bundler.bundle (/Users/mariacam/Development/html5-drag-and-drop/node_modules/parcel-bundler/src/Bundler.js:261:17)

Does parcel-bundler not work with npx???

@DeMoorJasper
Copy link
Member

@interglobalmedia are you using any plugins?

@interglobalmedia
Copy link

I have since removed parcel, but yes I was. If I remember correctly, the parcel-plugin-bundle-manifest, parcel-plugin-eslint, and parcel-plugin-static-files-copy.

@DeMoorJasper
Copy link
Member

@interglobalmedia then it’s definitely a plugin issue. Plugins shouldn’t be using internal parcel code (including the logger)

Sent with GitHawk

@interglobalmedia
Copy link

I'm not very familiar with parcel. What does that mean? And then why are there plugins to extend parcel? Thanks!

@DeMoorJasper
Copy link
Member

DeMoorJasper commented Dec 27, 2018

It means that parcel/src/Logger shouldn’t be imported directly but by using Bundler.logger instead as internal code is not protected by semver

The plugin architecture wasn’t perfect in parcel 1. It should be easier to extend functionality once Parcel 2 gets released.

The bug originates from this line probably https://github.com/BoltDoggy/parcel-plugin-eslint/blob/7a0947bdec7d4552a68ce3a3bbcba339040a2db2/index.js#L1

@interglobalmedia
Copy link

Thanks for the response! I looked for some kind of explanation about plugins in the docs, but didn't really find anything that might help. Just a list. Willing to give Parcel another go when version 2 comes out. But will hold off for the moment. I have things I need in my builds which don't work yet with Parcel 1+ (1.11.1). Looking forward to the release!

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