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

feat(build): create transparent builds #1198

Merged
merged 1 commit into from
Nov 7, 2019
Merged

Conversation

tripodsan
Copy link
Contributor

@tripodsan tripodsan commented Oct 30, 2019

fixes #1197

  • removed parcel dependency
  • faster builds
  • require source files intuitively

a simple html.htl with a html.pre.js is now transformed into:

.hlx/
└── build/
    └── src/
        ├── html.info.json
        ├── html.js
        ├── html.script.js
        └── html.script.js.map

with a generated:

html.js

...
const { pipe } = require('@adobe/helix-pipeline/src/defaults/html.pipe.js');
const { pre, before, after, replace } = require('../../../src/html.pre.js');
const script = require('./html.script.js');
...

the html.htl was transformed into html.script.js.


in case of a pure script, like xml.js with a custom xml.pipe.js:

.hlx/
└── build/
    └── src/
        ├── xml.info.json
        └── xml.js

with a generated:

xml.js

...
const { pipe } = require('../../../src/xml.pipe.js');
const { pre, before, after, replace } = require('@adobe/helix-pipeline/src/defaults/xml.pre.js');
const script = require('./../../../src/xml.js');
...

@tripodsan tripodsan changed the title feat(build): create transparent builds WIP: feat(build): create transparent builds Oct 30, 2019
@codecov
Copy link

codecov bot commented Oct 31, 2019

Codecov Report

Merging #1198 into master will decrease coverage by 0.26%.
The diff coverage is 97.4%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1198      +/-   ##
==========================================
- Coverage   92.18%   91.92%   -0.27%     
==========================================
  Files          44       38       -6     
  Lines        1855     1795      -60     
==========================================
- Hits         1710     1650      -60     
  Misses        145      145
Impacted Files Coverage Δ
src/build.cmd.js 100% <100%> (ø) ⬆️
src/package.cmd.js 89.79% <100%> (-2.52%) ⬇️
src/up.cmd.js 87.11% <100%> (-0.08%) ⬇️
src/builder/ActionBundler.js 91.3% <100%> (ø)
src/builder/Builder.js 96.96% <96.96%> (ø)

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 916a701...980f982. Read the comment docs.

@tripodsan tripodsan force-pushed the transparent-builds branch 4 times, most recently from ba7afeb to 494532a Compare October 31, 2019 13:19
@tripodsan tripodsan changed the title WIP: feat(build): create transparent builds feat(build): create transparent builds Oct 31, 2019
@tripodsan tripodsan marked this pull request as ready for review October 31, 2019 13:25
@alexkli
Copy link
Contributor

alexkli commented Nov 1, 2019

LGTM. The ../.. is currently a bit tricky for wskdebug because it mounts the .hlx/build folder into the docker container, where ../ doesn’t work, but maybe an option can be added to handle this (base dir vs. entry point file).

@tripodsan
Copy link
Contributor Author

LGTM. The ../.. is currently a bit tricky for wskdebug because it mounts the .hlx/build folder into the docker container, where ../ doesn’t work, but maybe an option can be added to handle this (base dir vs. entry point file).

yes please, i'd rather not copy the source files to the build folder again....

kptdobe
kptdobe previously approved these changes Nov 1, 2019
@kptdobe
Copy link
Contributor

kptdobe commented Nov 1, 2019

I assume all the existing tests validate that the build works as before.

@tripodsan
Copy link
Contributor Author

I assume all the existing tests validate that the build works as before.

yes...some of the tests were modified and the JSX test is skipped. Also, since we don't use parcel anymore, building no longer validates the sources.

Copy link
Contributor

@trieloff trieloff left a comment

Choose a reason for hiding this comment

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

What would be needed to keep JSX support?

@tripodsan
Copy link
Contributor Author

What would be needed to keep JSX support?

Some sort of JSX processor... but I would (re)add it, once someone asks about it :-)

@trieloff
Copy link
Contributor

trieloff commented Nov 5, 2019

The JSX processing is done by babel, which is still in the dependency tree:

"@babel/core": {

@tripodsan
Copy link
Contributor Author

The JSX processing is done by babel, which is still in the dependency tree:

I cleaned up the dependencies :-) but I'll see what can be done :-)

@tripodsan tripodsan force-pushed the transparent-builds branch 2 times, most recently from 8c6c722 to 7ddfb07 Compare November 6, 2019 02:46
@tripodsan tripodsan requested a review from trieloff November 6, 2019 02:51
@tripodsan
Copy link
Contributor Author

@trieloff I added JSX support again.... no more breaking change :-)

trieloff
trieloff previously approved these changes Nov 6, 2019
fixes #1197

- removed parcel dependency
- faster builds
- transparently require source files
@tripodsan tripodsan merged commit 6fbb1f1 into master Nov 7, 2019
@tripodsan tripodsan deleted the transparent-builds branch November 7, 2019 00:36
trieloff pushed a commit that referenced this pull request Nov 7, 2019
# [5.9.0](v5.8.5...v5.9.0) (2019-11-07)

### Features

* **build:** create transparent builds ([#1198](#1198)) ([6fbb1f1](6fbb1f1)), closes [#1197](#1197)
@adobe-bot
Copy link
Collaborator

🎉 This PR is included in version 5.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create transparent builds
5 participants