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

parcel v1 to v2 migration #150

Merged
merged 11 commits into from
May 26, 2021
Merged

parcel v1 to v2 migration #150

merged 11 commits into from
May 26, 2021

Conversation

Himavanth
Copy link
Contributor

@Himavanth Himavanth commented Apr 5, 2021

Description

TODO

  • Parcel cleanup before exit

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@Himavanth Himavanth requested a review from purplecabbage April 6, 2021 10:05
@Himavanth
Copy link
Contributor Author

@codecov
Copy link

codecov bot commented Apr 23, 2021

Codecov Report

Merging #150 (b2d6b7e) into master (6c11b0c) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #150   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          166       163    -3     
  Branches        39        39           
=========================================
- Hits           166       163    -3     
Impacted Files Coverage Δ
src/build-web.js 100.00% <100.00%> (ø)
src/bundle.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c11b0c...b2d6b7e. Read the comment docs.

@Himavanth Himavanth marked this pull request as ready for review May 5, 2021 09:19

const cleanup = async () => {
aioLogger.debug('cleanup bundler...')
await bundler.stop()
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this needs to be replaced with onBuildEnd event from the bundler for the cleanup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the onBuildEnd of Parcel V1, we have reporter plugin in V2. But i think it's different from the stop API. This event handler is to see what assets were built but not for cleanup.

Copy link
Member

@moritzraho moritzraho left a comment

Choose a reason for hiding this comment

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

some first comments, code looks good to me but need to test

src/bundle.js Show resolved Hide resolved
src/bundle.js Outdated
defaultConfig: require.resolve('@parcel/config-default'),
shouldDisableCache: false,
targets: {
action: {
Copy link
Member

Choose a reason for hiding this comment

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

why action is it some parcel jargon or you choose the target key ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can be anything.

Copy link
Member

Choose a reason for hiding this comment

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

ok should we use web-assets key instead ?

package.json Outdated Show resolved Hide resolved
@meryllblanchet meryllblanchet linked an issue May 19, 2021 that may be closed by this pull request
@Himavanth Himavanth merged commit fc39e7c into master May 26, 2021
@icaraps
Copy link

icaraps commented Jul 15, 2021

@Himavanth when trying to provide my custom parcel config with .parcelrcat the root of the project, I get a build failure

 Error: Invalid Parcel Config

My config looks like this:

{
  "extends": ["@parcel/config-default"],
  "reporters":  ["...", "parcel-reporter-static-files-copy"]
}

Do you know if I'm doing something wrong ? Or is it not possible anymore to provide a custom parcel config ?

@icaraps
Copy link

icaraps commented Jul 19, 2021

There is some mismatch in the latest tags of parcel and the associated npm packages. Parcel has beta.2 as latest but parcel/config-default has alpha.3.
Using npm i --save-dev @parcel/config-default@2.0.0-beta.3.1 I could get past the invalid parcel config thanks @Himavanth !

@icaraps
Copy link

icaraps commented Jul 22, 2021

@Himavanth I noticed the .parcel-cache folder is not gitignored by default... I think it should WDYT ?

@Himavanth
Copy link
Contributor Author

@icaraps good point. I will add it.

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.

evaluate moving to parcel v2
5 participants