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

Omit browserify-preprocessor from dependencies #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

peterbee
Copy link

This reverts a previous change to add @cypress/browserify-preprocessor to this package's dependencies. It also clarifies usage in the README for Clojurescript-only and CLJS + JS environments.

⚠️ I had trouble testing this locally. I could not discern if my troubles were related to this change or just with npm linking.

@viesti
Copy link
Owner

viesti commented Jan 30, 2021

Sorry, have been a bit lazy with this :/ Seems that since last summer when hacking on this, Cypress was on version 4.x, now it's on version 6.x (!). Also seems that version 5.0.0 switched to using Webpack preprocessor by default: https://docs.cypress.io/guides/references/changelog.html#5-0-0

Cypress no longer supports file paths with a question mark ? or exclamation mark ! in them. We now use the webpack preprocessor by default and it does not support files with question marks or exclamation marks. Addressed in #7982.

So it makes even more sense to remove the dependency to browserify preprocessor I guess :)

If I remember correctly, the problem was that if one does not delegate to the default preprocessor, the file is not preprocessed at all, which is a problem for even the sample code of a fresh cypress install, since the samples use code that requires preprocessing. So if a custom preprocessor doesn't handle a file, the fallback has to be explicitly written.

The makeBrowserifyPreprocessor() reference in the readme in this PR relies on makeBrowserifyPreprocessor being available. The original readme had a more full example, on installing the cypress/browserify-preprocessor dependency and making the helper available via

const makeBrowserifyPreprocessor = require('@cypress/browserify-preprocessor');

What is a bit funny that I now don't remember how one could fallback to use the default preprocessor bundled inside Cypress installation. Might be good to look if there is an instruction for that (maybe I missed) or if not, hop onto the Cypress 6.x time and even use the webpack preprocessor (https://github.com/cypress-io/cypress/tree/master/npm/webpack-preprocessor), since this is what new projects would probably use.

@viesti
Copy link
Owner

viesti commented Feb 2, 2021

Hmm, haven't looked how other preprocessors work that much, but this made me wonder that is the conditional check even necessary, assuming that each preprocessor checks if a file should be processed: cypress-io/cypress#5832 (comment)

@viesti viesti mentioned this pull request Sep 2, 2021
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.

2 participants