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

implement as transform #2

Merged
merged 1 commit into from
Jan 25, 2018
Merged

implement as transform #2

merged 1 commit into from
Jan 25, 2018

Conversation

fcurella
Copy link
Contributor

@fcurella fcurella commented Jan 23, 2018

I took the original code and implemented it as browserify transform. This way we don't need to write temporary files to disk and we can leverage @cypress/browserify-preprocessor's handling of watchers and reloading. (ref #1)

I've also made the preprocessor accept options to be passed down to browserify.

Note: I've used the airbnb style simply because that's what I usually have set up. If you prefer a different style, let me know and I'll make the changes (maybe add an .eslintrc to the repo?)

index.js Outdated


const preprocessor = (options) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

do you have something against using (options = browserify.defaultOptions) here instead? I think this way we could keep the backward compatibility and make it as easy to set up as it already was (and if someone wants to customise - there is a way).
Otherwise - great work :) this makes this plugin really usable :) and we are still just at the beginning of the journey

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, I'll make the change

@badeball
Copy link
Owner

I have a question about this. My understanding is that @cypress/browserify-preprocessor already takes care of watching. When trying to re-implement this, I tried omitting watching completely and I am observing identical behavior with my fork. Thus, why is chokidar necessary here?

@lgandecki
Copy link
Collaborator

Are your tests restarting when either .feature file or your step definitions change? Or only when your .feature file changes? I think that was the point here. Maybe it's started behaving differently comparing to 2018?

@badeball
Copy link
Owner

Are your tests restarting when either .feature file or your step definitions change?

Both!

Maybe it's started behaving differently comparing to 2018?

I thought that could be it, but they had watchify in v1.02 as well, seen here.

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.

3 participants