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

Feature(transformer): re-implement postProcess API #8026

Merged

Conversation

Shinyaigeek
Copy link
Contributor

@Shinyaigeek Shinyaigeek commented Apr 29, 2022

↪️ Pull Request

Part of #7503

postProcess API was removed in #5674 because this API was unused, but this API is necessary for svelte preprocessing, so I start by re-implement this postProcess API in order to accomplish no-config svelte support.

🚨 Test instructions

Transformer’s API, such as transformer’s generate method, does not seems to be tested directly and a testing method does not seem to be established. In addition to these reasons, I thought Transformer’s API is tested indirectly by much of integration-test, so I did not write any unit-test about this API, but I think it would be better to write the unit-test about postProcess, so I will write a test if you want.

✔️ 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

@@ -186,7 +186,7 @@ export type EnvironmentOptions = {|
*/
export type VersionMap = {
[string]: string,
...,
...
Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems to be a change by the formatter, which runs pre-commit

/** Do some processing after the transformation */
postProcess?: ({|
assets: Array<MutableAsset>,
config: ConfigType,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

other than config type on this line and some comments and some invalidation logic are completely same to https://github.com/parcel-bundler/parcel/pull/5674/files

@Shinyaigeek Shinyaigeek force-pushed the feature/re-implement-postProcess-api branch from c770b2e to 8799a32 Compare April 29, 2022 16:57
@devongovett
Copy link
Member

Once we have something that uses this, we should add some caching tests for this. One possible issue is if a transformer plugin returns a different list of assets from postProcess, the invalidations/dependencies in the original assets will be lost resulting in the cache/watcher not being invalidated on changes. Not sure if we should/can do this automatically in core, or whether we need to leave that to plugins. We'll see as this progresses further.

@devongovett devongovett merged commit e51955f into parcel-bundler:v2 May 9, 2022
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

3 participants